emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
@ 2014-08-05 21:45 Federico Beffa
  2014-08-09  8:00 ` Nicolas Goaziou
  0 siblings, 1 reply; 16+ messages in thread
From: Federico Beffa @ 2014-08-05 21:45 UTC (permalink / raw)
  To: rasmus, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]

>>> The current proposal is to make them elements instead of objects in Org
>>> syntax (i.e, a `latex-environment' instead of a `latex-fragment'). In
>>> a nutshell:
>>>
>>>  - Pros:
>>>    + conform to LaTeX intent,
>>>    + impossible to fill.
>>>  - Cons:
>>>    - documents containing \[...\] mid-line will be broken (such
>>>      constructs will not be recognized anymore).

...

> If this is always the case for you, you can "fix" this behavior by
> always putting it in a new paragraph and using a filter.

OK, I understand that the Cons above is a serious issue and nobody wants it.
For this reason I went back to my original idea and modified the
`org-fill-paragraph' function. This provides the two Pros above
without having to introduce the undesired Cons. The behavior is a
follows:
- if \[...\] is inline, behave as before.
- if \[ is the first non space character of a line and the closing \]
is the last non space character of a line (possibly spanning several
lines), then do not fill this region of the paragraph.

Attached you find a patch with the proposed modification. I would
greatly appreciate if you could consider it for inclusion in org-mode
and provide feedback.

I understand that if you are willing to make use of this patch, I will
have to sign a copyright assignment form. This is not a problem, but
please do not make me waste time with forms if you do not want to
include the patch in org-mode. Please provide instructions on how to
proceed.

Regards,
Federico.

[-- Attachment #2: 0001-org.el-make-org-paragraph-fill-ignore-.-regions-star.patch --]
[-- Type: application/octet-stream, Size: 6073 bytes --]

From 13f2b01e2c7b2dc7d11c7c90640d075f02fcc6c3 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Tue, 5 Aug 2014 23:08:26 +0200
Subject: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions
 starting and ending a line

* lisp/org.el (org-paragraph-fill): If a LaTeX \[...\] macro starts
a line and ends a line (possibly spanning multiple lines), then do not
change that part of the paragraph.
---
 lisp/org.el |  128 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 113 insertions(+), 15 deletions(-)
 mode change 100644 => 100755 lisp/org.el

diff --git a/lisp/org.el b/lisp/org.el
old mode 100644
new mode 100755
index 0f7a4ef..6cb0cd3
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -22775,6 +22775,118 @@ matches in paragraphs or comments, use it."
 
 (declare-function message-goto-body "message" ())
 (defvar message-cite-prefix-regexp)	; From message.el
+
+(defun org-paragraph-find-dmr (beg end)
+  "Find \\=\\[...\\=\\] LaTeX macros starting and ending a line.
+
+This function returns a list of the form ((dm1-beg dm1-end)
+... (dmN-beg dmN-end)) with dmi-beg and dmi-end, i=1,...,N
+denoting the start respectively the end positions of the i-th
+\\=\\[...\\=\\] display math LaTeX macros starting and ending a
+line (possibly spanning multiple-lines).  This means that \\=\\[
+must be the first non space character of a line and \\=\\] must
+the the last non space character of a line.  The two can be on
+different lines."
+  (let ((dmrl nil) (dmr nil))
+    (save-excursion
+      (goto-char end)
+      (while (setq dmr 
+		   (if
+		       (re-search-backward 
+			(concat 
+			 "\\(^[ \t]*\\\\\\[\\).*\\([\r]?\n\\)*"
+			 "\\(.+[\r]?\n\\)*"
+			 ".*\\\\\\]"
+			 "[ \t]*[\r]?\n") beg t)
+		       (list (match-beginning 1) (match-end 0))
+		     nil))
+	(setq dmrl (append (list dmr) dmrl))))
+    dmrl))
+
+(defun org-paragraph-find-line-breaks (beg end)
+  "Find org `line-break' objects in a paragraph.
+
+Find the positions of org `line-break' objects and returns a list
+with their position, including BEG and END."
+  (append 
+   (cons beg
+	 (org-element-map
+	     (org-element--parse-objects
+	      beg end nil (org-element-restriction 'paragraph))
+	     'line-break
+	   (lambda (lb) (org-element-property :end lb))))
+   (list end)))
+
+(defun org-fill-paragraph-construct-regions (lbl dmrl)
+  "Construct paragraph regions to be filled.
+
+This function takes a list LBL with the positions of org
+`line-break' objects and a list DMRL with the start and end
+positions of \\=\\[...\\=\\] LaTeX macros beginning and ending a
+line.  It returns a list of the form ((r1-beg r1-end) ... (rN-beg
+rN-end)) with the start end end positions of the paragraph
+regions to be filled."
+  (let ((lbl-new lbl))
+    ;; first we make sure there are no line-breaks within displayed
+    ;; math regions
+    (dolist (lb-elt lbl)
+      (mapc (lambda (x) 
+	      (when (and (> lb-elt (nth 0 x)) (< lb-elt (nth 1 x)))
+		(setq lbl-new (remove lb-elt lbl-new))))
+	    dmrl)
+      )
+    ;; then we construct the regions to be filled
+    (let ((lb-this (car lbl-new)) (dmrl-local dmrl) 
+	  dmr-elt regions lb-done)
+      (dolist (lb-next (cdr lbl-new))
+	(setq lb-done nil)
+	;; we have to keep the current line break until we put it at
+	;; the right place, possibly after some displayed math regions
+	(while (not lb-done)
+	  (if (setq dmr-elt (car dmrl-local))
+	    (cond 
+	     ;; line break before next displayed math region
+	     ((< lb-next (nth 0 dmr-elt))
+	      (setq regions (append regions (list (list lb-this lb-next))))
+	      (setq lb-done t)
+	      (setq lb-this lb-next))
+	     ;; line break after display math region
+	     (t
+	      (setq regions (append 
+			     regions 
+			     (list (list lb-this (nth 0 dmr-elt)))))
+	      (setq lb-this (nth 1 dmr-elt))
+	      (setq dmrl-local (cdr dmrl-local))
+	      (setq lb-done nil)))
+	    ;; no displayed math regions left
+	    (progn 
+	      (setq lb-done t)
+	      ;; if a displayed math region finishes a paragraph we do
+	      ;; not need to fill the region after the ending
+	      ;; displayed math region.
+	      (unless (eq lb-this lb-next)
+		(setq regions (append 
+			       regions 
+			       (list (list lb-this lb-next)))))
+	      (setq lb-this lb-next)))))
+      regions)))
+
+(defun org-fill-paragraph-regions (beg end justify)
+  "Fill paragraph starting at BEG and ending at END.
+
+This function is called by `org-fill-paragraph' to fill a
+paragraph.  If a LaTeX \\=\\[...\\=\\] macro starts a line and
+ends a line (possibly spanning multiple lines), then do not
+change that part of the paragraph. Respect org `line-break'
+objects."
+  (let* ((dmrl (org-paragraph-find-dmr beg end))
+	(lbl (org-paragraph-find-line-breaks beg end))
+	(fill-regions (org-fill-paragraph-construct-regions lbl dmrl)))
+    ;; fill the various regions starting from the last one
+    (mapc (lambda (r) 
+	    (fill-region-as-paragraph (nth 0 r) (nth 1 r) justify))
+	  (nreverse fill-regions))))
+
 (defun org-fill-paragraph (&optional justify)
   "Fill element at point, when applicable.
 
@@ -22846,21 +22958,7 @@ a footnote definition, try to fill the first paragraph within."
 	       ;; separators, and fill the parts in reverse order to
 	       ;; avoid messing with markers.
 	       (save-excursion
-		 (goto-char end)
-		 (mapc
-		  (lambda (pos)
-		    (fill-region-as-paragraph pos (point) justify)
-		    (goto-char pos))
-		  ;; Find the list of ending positions for line breaks
-		  ;; in the current paragraph.  Add paragraph
-		  ;; beginning to include first slice.
-		  (nreverse
-		   (cons beg
-			 (org-element-map
-			     (org-element--parse-objects
-			      beg end nil (org-element-restriction 'paragraph))
-			     'line-break
-			   (lambda (lb) (org-element-property :end lb)))))))
+		 (org-fill-paragraph-regions beg end justify))
 	       t)))
 	  ;; Contents of `comment-block' type elements should be
 	  ;; filled as plain text, but only if point is within block
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
@ 2014-08-07 13:56 Federico Beffa
  0 siblings, 0 replies; 16+ messages in thread
From: Federico Beffa @ 2014-08-07 13:56 UTC (permalink / raw)
  To: emacs-orgmode

> Attached you find a patch with the proposed modification. I would
> greatly appreciate if you could consider it for inclusion in org-mode
> and provide feedback.

Here a more lispy version of the function
`org-fill-paragraph-construct-regions' used in the patch.I guess it
could be more appealing to people on this list :-)

(defun org-fill-paragraph-construct-regions (lbl dmrl)
  "Construct paragraph regions to be filled.

This function takes an ordered list LBL with the positions of org
`line-break' objects and an ordered list DMRL with the start and
end positions of \\=\\[...\\=\\] LaTeX macros beginning and
ending a line.  It returns a list of the form ((r1-beg r1-end)
... (rN-beg rN-end)) with the start end end positions of the
paragraph regions to be filled."
  (let ((lbl-len (length lbl))) ; compute only once length of lbl
    (or
     ;; elementary case 1: no display math regions and 2 entries in lbl
     (and (not dmrl)
      (eq lbl-len 2)
      (list lbl))
     ;; elementary case 2: 1 remaining line break (end of paragraph) and
     ;; 1 remaining display math region.
     (and (eq (length dmrl) 1)
      (eq lbl-len 1)
      (list (list (nth 1 dmrl) (car lbl))))
     ;; remove line-breaks within display math regions
     (and dmrl (>= (nth 1 lbl) (caar dmrl))
      (<= (nth 1 lbl) (nth 1 (car dmrl)))
      (if (> lbl-len 2)
          (org-fill-paragraph-construct-regions2
           (cons (car lbl) (cddr lbl))
           dmrl)
        ;; a displayed math region finished the paragraph
        (org-fill-paragraph-construct-regions2
         (cons (car lbl) (list (caar dmrl)))
         nil)))
     ;; non elementary cases:
     (if (and dmrl (> (nth 1 lbl) (caar dmrl)))
     (cons (list (car lbl) (caar dmrl))
           (org-fill-paragraph-construct-regions2
        (cons (nth 1 (car dmrl)) (cdr lbl))
        (cdr dmrl)))
       (cons (list (car lbl) (nth 1 lbl))
         (org-fill-paragraph-construct-regions2
          (cdr lbl)
          dmrl))))))

Regards,
Federico

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-05 21:45 Federico Beffa
@ 2014-08-09  8:00 ` Nicolas Goaziou
  2014-08-09 10:20   ` Federico Beffa
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-09  8:00 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode, rasmus

Hello,

Federico Beffa <beffa@ieee.org> writes:

> OK, I understand that the Cons above is a serious issue and nobody wants it.
> For this reason I went back to my original idea and modified the
> `org-fill-paragraph' function. This provides the two Pros above
> without having to introduce the undesired Cons. The behavior is a
> follows:
> - if \[...\] is inline, behave as before.
> - if \[ is the first non space character of a line and the closing \]
> is the last non space character of a line (possibly spanning several
> lines), then do not fill this region of the paragraph.

But then, you introduce a discrepancy between M-q results and Org
syntax. This is not optimal either.

Also, this will not be consistent anyway with \begin{...} ... \end{...}
markup, which is handled differently, unlike to LaTeX.

> Attached you find a patch with the proposed modification. I would
> greatly appreciate if you could consider it for inclusion in org-mode
> and provide feedback.

Note that you should use master to provide patches. `org-fill-paragraph'
was modified recently.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-09  8:00 ` Nicolas Goaziou
@ 2014-08-09 10:20   ` Federico Beffa
  2014-08-09 23:20     ` Nicolas Goaziou
  0 siblings, 1 reply; 16+ messages in thread
From: Federico Beffa @ 2014-08-09 10:20 UTC (permalink / raw)
  To: Rasmus, emacs-orgmode, Nicolas Goaziou

>> without having to introduce the undesired Cons. The behavior is a
>> follows:
>> - if \[...\] is inline, behave as before.
>> - if \[ is the first non space character of a line and the closing \]
>> is the last non space character of a line (possibly spanning several
>> lines), then do not fill this region of the paragraph.
>
> But then, you introduce a discrepancy between M-q results and Org
> syntax. This is not optimal either.

Would you mind elaborating on the discrepancy. I do not quite
understand what do you mean. Doesn't the Org syntax "just" defines the
result of parsing a buffer (like in most programming languages where
indentation and line breaks are just to help people reading code)?

>
> Also, this will not be consistent anyway with \begin{...} ... \end{...}
> markup, which is handled differently, unlike to LaTeX.

Yes, I agree. But, given that we do not want to make \[...\] an
`org-mode' environment for backward compatibility reasons, this
appears to be the only alternative. Note that (as noted earlier)
`latex-mode' behaves similarly: it does not fill parts of a paragraph
where a line begins with \[ and ends with \]. The reason and goal is
to achieve that a carefully typed long equation does not get scrambled
by M-q because if it does it becomes difficult to read for humans.

>
>> Attached you find a patch with the proposed modification. I would
>> greatly appreciate if you could consider it for inclusion in org-mode
>> and provide feedback.
>
> Note that you should use master to provide patches. `org-fill-paragraph'
> was modified recently.

OK, will pull again.

Regards,
Federico

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-09 10:20   ` Federico Beffa
@ 2014-08-09 23:20     ` Nicolas Goaziou
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-09 23:20 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode, Rasmus

Federico Beffa <beffa@ieee.org> writes:

>> But then, you introduce a discrepancy between M-q results and Org
>> syntax. This is not optimal either.
>
> Would you mind elaborating on the discrepancy.

Ignoring \[...\] when filling the paragraph is misleading. You may
believe the object doesn't belong to the paragraph at all. I think M-q
should, on the contrary, give clues about the structure of the document.

Also, it doesn't make a difference when exporting to LaTeX, but it might
in back-ends with a different definition for paragraphs (e.g. HTML).

> I do not quite understand what do you mean. Doesn't the Org syntax
> "just" defines the result of parsing a buffer (like in most
> programming languages where indentation and line breaks are just to
> help people reading code)?

Indentation and line breaks are meaningful in Org. They are not just
syntactic sugar.

> Yes, I agree. But, given that we do not want to make \[...\] an
> `org-mode' environment for backward compatibility reasons, this
> appears to be the only alternative.

There is another alternative: use \begin{equation*}
or \begin{displaymath}. M-q does not fill those.

You're trying to solve an already solved problem, although the actual
solution doesn't suit you.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
@ 2014-08-10 13:13 Federico Beffa
  2014-08-11 13:06 ` Nicolas Goaziou
  0 siblings, 1 reply; 16+ messages in thread
From: Federico Beffa @ 2014-08-10 13:13 UTC (permalink / raw)
  To: mail, emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Ignoring \[...\] when filling the paragraph is misleading. You may
> believe the object doesn't belong to the paragraph at all. I think M-q
> should, on the contrary, give clues about the structure of the document.
>
> Also, it doesn't make a difference when exporting to LaTeX, but it might
> in back-ends with a different definition for paragraphs (e.g. HTML).

to help me understand what kind of problems one could face with HTML (or
another back-end), could you give a concrete example?

>
>> I do not quite understand what do you mean. Doesn't the Org syntax
>> "just" defines the result of parsing a buffer (like in most
>> programming languages where indentation and line breaks are just to
>> help people reading code)?
>
> Indentation and line breaks are meaningful in Org. They are not just
> syntactic sugar.

Sorry, I didn't express myself correctly. I guess within a paragraph
indentation and line breaks are syntactic sugar. Right?

Regards,
Federico

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
@ 2014-08-10 13:15 Federico Beffa
  0 siblings, 0 replies; 16+ messages in thread
From: Federico Beffa @ 2014-08-10 13:15 UTC (permalink / raw)
  To: mail, emacs-orgmode; +Cc: Rasmus

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Ignoring \[...\] when filling the paragraph is misleading. You may
> believe the object doesn't belong to the paragraph at all. I think M-q
> should, on the contrary, give clues about the structure of the document.
>
> Also, it doesn't make a difference when exporting to LaTeX, but it might
> in back-ends with a different definition for paragraphs (e.g. HTML).

to help me understand what kind of problems one could face with HTML (or
another back-end), could you give a concrete example?

>
>> I do not quite understand what do you mean. Doesn't the Org syntax
>> "just" defines the result of parsing a buffer (like in most
>> programming languages where indentation and line breaks are just to
>> help people reading code)?
>
> Indentation and line breaks are meaningful in Org. They are not just
> syntactic sugar.

Sorry, I didn't express myself correctly. I guess within a paragraph
indentation and line breaks are syntactic sugar. Right?

Regards,
Federico

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-10 13:13 Federico Beffa
@ 2014-08-11 13:06 ` Nicolas Goaziou
  2014-08-11 18:27   ` Federico Beffa
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-11 13:06 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode

Federico Beffa <beffa@ieee.org> writes:

> to help me understand what kind of problems one could face with HTML (or
> another back-end), could you give a concrete example?

line 1
line 2
\[1+1\]

==>

<p>
line 1
line 2
<img... />
</p>

whereas

line 1
line 2
\begin{equation*}
1+1
\end{equation*}

==>

<p>
line 1
line 2
</p>

<p>
<img ... />
</p>

IOW, there are two different structures in the document:

  ((paragraph latex))

  vs

  ((paragraph) (latex))

even though M-q cannot tell that difference (with your proposal, the
behaviour would be the same in both cases).


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 13:06 ` Nicolas Goaziou
@ 2014-08-11 18:27   ` Federico Beffa
  2014-08-11 19:43     ` Rasmus
  2014-08-11 20:40     ` Nicolas Goaziou
  0 siblings, 2 replies; 16+ messages in thread
From: Federico Beffa @ 2014-08-11 18:27 UTC (permalink / raw)
  To: emacs-orgmode, Nicolas Goaziou; +Cc: Rasmus

On Mon, Aug 11, 2014 at 3:06 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
> Federico Beffa <beffa@ieee.org> writes:
>
>> to help me understand what kind of problems one could face with HTML (or
>> another back-end), could you give a concrete example?
>
> line 1
> line 2
> \[1+1\]
>
> ==>
>
> <p>
> line 1
> line 2
> <img... />
> </p>
>
> whereas
>
> line 1
> line 2
> \begin{equation*}
> 1+1
> \end{equation*}
>
> ==>
>
> <p>
> line 1
> line 2
> </p>
>
> <p>
> <img ... />
> </p>
>
> IOW, there are two different structures in the document:
>
>   ((paragraph latex))
>
>   vs
>
>   ((paragraph) (latex))
>
> even though M-q cannot tell that difference (with your proposal, the
> behaviour would be the same in both cases).

The example highlight the difference that I suggested to remove in the
very first place (by making \[...\] an environment). This was rejected
to preserve backward compatibility and that's fine. So I moved on to a
second proposal: modify the paragraph filling function.

Didn't the following comment in your previous reply refer to the
second proposal?

> Ignoring \[...\] when filling the paragraph is misleading. You may
> believe the object doesn't belong to the paragraph at all. I think M-q
> should, on the contrary, give clues about the structure of the document.
>
> Also, it doesn't make a difference when exporting to LaTeX, but it might
> in back-ends with a different definition for paragraphs (e.g. HTML).

From your sentence in your last reply:
   "... even though M-q cannot tell that difference (with your proposal,
   the behaviour would be the same in both cases)."
I understand that there is no technical deficiency in it. Am I
therefore correct in saying that if you prefer not to include this
proposal in org-mode it isn't for technical reasons but it is a matter
of opinion/taste?

I would like to know, because in any case I would like to use the
proposed filling function in my copy of org-mode and if you see
technical problems/errors I very much would like to be aware of them
and, if possible, avoid/correct them.

Regards,
Federico

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 18:27   ` Federico Beffa
@ 2014-08-11 19:43     ` Rasmus
  2014-08-11 20:44       ` Nicolas Goaziou
  2014-08-11 20:40     ` Nicolas Goaziou
  1 sibling, 1 reply; 16+ messages in thread
From: Rasmus @ 2014-08-11 19:43 UTC (permalink / raw)
  To: beffa; +Cc: emacs-orgmode, mail

Federico Beffa <beffa@ieee.org> writes:

> On Mon, Aug 11, 2014 at 3:06 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
>> Federico Beffa <beffa@ieee.org> writes:
>>
>
>>> to help me understand what kind of problems one could face with HTML (or
>>> another back-end), could you give a concrete example?
>>
>> line 1
>> line 2
>> \[1+1\]
>>
>> ==>
>>
>> <p>
>> line 1
>> line 2
>> <img... />
>> </p>
>>
>> whereas
>>
>> line 1
>> line 2
>> \begin{equation*}
>> 1+1
>> \end{equation*}
>>
>> ==>
>>
>> <p>
>> line 1
>> line 2
>> </p>
>>
>> <p>
>> <img ... />
>> </p>
>>
>> IOW, there are two different structures in the document:
>>
>>   ((paragraph latex))
>>
>>   vs
>>
>>   ((paragraph) (latex))
>>
>> even though M-q cannot tell that difference (with your proposal, the
>> behaviour would be the same in both cases).
>
> The example highlight the difference that I suggested to remove in the
> very first place (by making \[...\] an environment). This was rejected
> to preserve backward compatibility and that's fine. So I moved on to a
> second proposal: modify the paragraph filling function.
>
> Didn't the following comment in your previous reply refer to the
> second proposal?
>
>> Ignoring \[...\] when filling the paragraph is misleading. You may
>> believe the object doesn't belong to the paragraph at all. I think M-q
>> should, on the contrary, give clues about the structure of the document.
>>
>> Also, it doesn't make a difference when exporting to LaTeX, but it might
>> in back-ends with a different definition for paragraphs (e.g. HTML).
>
> From your sentence in your last reply:
>    "... even though M-q cannot tell that difference (with your proposal,
>    the behaviour would be the same in both cases)."
> I understand that there is no technical deficiency in it. Am I
> therefore correct in saying that if you prefer not to include this
> proposal in org-mode it isn't for technical reasons but it is a matter
> of opinion/taste?
>
> I would like to know, because in any case I would like to use the
> proposed filling function in my copy of org-mode and if you see
> technical problems/errors I very much would like to be aware of them
> and, if possible, avoid/correct them.

How about adding the possibility to add hooks to org-fill-paragraph?
So that people can add "extensions" to fill as they want and the core
function can rely on org-element only?

—Rasmus

-- 
Lasciate ogni speranza, voi che leggete questo.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 18:27   ` Federico Beffa
  2014-08-11 19:43     ` Rasmus
@ 2014-08-11 20:40     ` Nicolas Goaziou
  2014-08-16  7:38       ` Federico Beffa
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-11 20:40 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode, Rasmus

Federico Beffa <beffa@ieee.org> writes:

> The example highlight the difference that I suggested to remove in the
> very first place (by making \[...\] an environment). This was rejected
> to preserve backward compatibility and that's fine. So I moved on to a
> second proposal: modify the paragraph filling function.

This example illustrates that it would be nice to fill two different
structures, well, differently.

> From your sentence in your last reply:
>    "... even though M-q cannot tell that difference (with your proposal,
>    the behaviour would be the same in both cases)."
> I understand that there is no technical deficiency in it.

I cannot speak about technical deficiency. I didn't look closely at the
code. There are some gotchas, though, e.g., when auto-filling (moving
from "inline" to "block").

> Am I therefore correct in saying that if you prefer not to include
> this proposal in org-mode it isn't for technical reasons but it is
> a matter of opinion/taste?

I'm just pointing out an ergonomy (or consistency) annoyance in your
proposal. I'm not thrilled by faking the filling mechanism.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 19:43     ` Rasmus
@ 2014-08-11 20:44       ` Nicolas Goaziou
  2014-08-16  7:50         ` Federico Beffa
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-11 20:44 UTC (permalink / raw)
  To: Rasmus; +Cc: emacs-orgmode, beffa

Rasmus <rasmus@gmx.us> writes:

> How about adding the possibility to add hooks to org-fill-paragraph?
> So that people can add "extensions" to fill as they want and the core
> function can rely on org-element only?

You can `defadvice' anything to your heart's content.

OTOH, providing a hook for such a core feature is just
over-engineering.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 20:40     ` Nicolas Goaziou
@ 2014-08-16  7:38       ` Federico Beffa
  2014-08-28 10:10         ` Nicolas Goaziou
  0 siblings, 1 reply; 16+ messages in thread
From: Federico Beffa @ 2014-08-16  7:38 UTC (permalink / raw)
  To: emacs-orgmode, Rasmus, Nicolas Goaziou

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

On Mon, Aug 11, 2014 at 10:40 PM, Nicolas Goaziou
> I'm just pointing out an ergonomy (or consistency) annoyance in your
> proposal. I'm not thrilled by faking the filling mechanism.

Well, that has nothing to do with consistency (neither with ergonomy,
unless you produce a large scale statistical study on the subject).
First, the proposed change modifies a user function which is not
supposed to be used as debugging tool for the parsing of a document.
For that you have other tools. Second, the modified function still
fills the whole paragraph and does not stop at a \[...\] block. It
just does not scramble the content of such a block.

It's really a matter of opinions: you perceive this as introducing an
"inconsistency". I see this as adapting a function to better suit the
situation. And, given that latex-mode behaves in the same way, I'm for
sure not the only one looking at this in this way.

In any case, I've updated my git repository (less than 24h ago) and
recreated the patch. I've tested it and believe it is working. Why
don't you give it a try. It's not such a disruptive change.

Regards,
Fede

[-- Attachment #2: 0001-org.el-make-org-paragraph-fill-ignore-.-regions-star.patch --]
[-- Type: application/octet-stream, Size: 5682 bytes --]

From 7c3dfca0d3ea447bfd40f810a197635b1bca9e86 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Tue, 5 Aug 2014 23:08:26 +0200
Subject: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions
 starting and ending a line

* lisp/org.el (org-paragraph-fill): If a LaTeX \[...\] macro starts
a line and ends a line (possibly spanning multiple lines), then do not
change that part of the paragraph.
---
 lisp/org.el |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 101 insertions(+), 15 deletions(-)
 mode change 100644 => 100755 lisp/org.el

diff --git a/lisp/org.el b/lisp/org.el
old mode 100644
new mode 100755
index 0f7a4ef..dfd4a89
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -22775,6 +22775,106 @@ matches in paragraphs or comments, use it."
 
 (declare-function message-goto-body "message" ())
 (defvar message-cite-prefix-regexp)	; From message.el
+
+(defun org-paragraph-find-dmr (beg end)
+  "Find \\=\\[...\\=\\] LaTeX macros starting and ending a line.
+
+This function returns a list of the form ((dm1-beg dm1-end)
+... (dmN-beg dmN-end)) with dmi-beg and dmi-end, i=1,...,N
+denoting the start respectively the end positions of the i-th
+\\=\\[...\\=\\] display math LaTeX macros starting and ending a
+line (possibly spanning multiple-lines).  This means that \\=\\[
+must be the first non space character of a line and \\=\\] must
+the the last non space character of a line.  The two can be on
+different lines."
+  (let ((dmrl nil) (dmr nil))
+    (save-excursion
+      (goto-char end)
+      (while (setq dmr 
+		   (if
+		       (re-search-backward 
+			(concat 
+			 "\\(^[ \t]*\\\\\\[\\).*\\([\r]?\n\\)*"
+			 "\\(.+[\r]?\n\\)*"
+			 ".*\\\\\\]"
+			 "[ \t]*[\r]?\n") beg t)
+		       (list (match-beginning 1) (match-end 0))
+		     nil))
+	(setq dmrl (append (list dmr) dmrl))))
+    dmrl))
+
+(defun org-paragraph-find-line-breaks (beg end)
+  "Find org `line-break' objects in a paragraph.
+
+Find the positions of org `line-break' objects and returns a list
+with their position, including BEG and END."
+  (append 
+   (cons beg
+	 (org-element-map
+	     (org-element--parse-objects
+	      beg end nil (org-element-restriction 'paragraph))
+	     'line-break
+	   (lambda (lb) (org-element-property :end lb))))
+   (list end)))
+
+(defun org-fill-paragraph-construct-regions (lbl dmrl)
+  "Construct paragraph regions to be filled.
+
+This function takes an ordered list LBL with the positions of org
+`line-break' objects and an ordered list DMRL with the start and
+end positions of \\=\\[...\\=\\] LaTeX macros beginning and
+ending a line.  It returns a list of the form ((r1-beg r1-end)
+... (rN-beg rN-end)) with the start end end positions of the
+paragraph regions to be filled."
+  (let ((lbl-len (length lbl))) ; compute only once length of lbl
+    (or
+     ;; elementary case 1: no display math regions and 2 entries in lbl
+     (and (not dmrl)
+	  (eq lbl-len 2)
+	  (list lbl))
+     ;; elementary case 2: 1 remaining line break (end of paragraph) and
+     ;; 1 remaining display math region.
+     (and (eq (length dmrl) 1)
+	  (eq lbl-len 1)
+	  (list (list (nth 1 dmrl) (car lbl))))
+     ;; remove line-breaks within display math regions
+     (and dmrl (>= (nth 1 lbl) (caar dmrl)) 
+	  (<= (nth 1 lbl) (nth 1 (car dmrl)))
+	  (if (> lbl-len 2)
+	      (org-fill-paragraph-construct-regions
+	       (cons (car lbl) (cddr lbl))
+	       dmrl)
+	    ;; a displayed math region finished the paragraph
+	    (org-fill-paragraph-construct-regions
+	     (cons (car lbl) (list (caar dmrl)))
+	     nil)))
+     ;; non elementary cases:
+     (if (and dmrl (> (nth 1 lbl) (caar dmrl)))
+	 (cons (list (car lbl) (caar dmrl))
+	       (org-fill-paragraph-construct-regions
+		(cons (nth 1 (car dmrl)) (cdr lbl))
+		(cdr dmrl)))
+       (cons (list (car lbl) (nth 1 lbl))
+	     (org-fill-paragraph-construct-regions
+	      (cdr lbl)
+	      dmrl))))))
+
+(defun org-fill-paragraph-regions (beg end justify)
+  "Fill paragraph starting at BEG and ending at END.
+
+This function is called by `org-fill-paragraph' to fill a
+paragraph.  If a LaTeX \\=\\[...\\=\\] macro starts a line and
+ends a line (possibly spanning multiple lines), then do not
+change that part of the paragraph.  Respect org `line-break'
+objects."
+  (let* ((dmrl (org-paragraph-find-dmr beg end))
+	(lbl (org-paragraph-find-line-breaks beg end))
+	(fill-regions (org-fill-paragraph-construct-regions lbl dmrl)))
+    ;; fill the various regions starting from the last one
+    (mapc (lambda (r) 
+	    (fill-region-as-paragraph (nth 0 r) (nth 1 r) justify))
+	  (nreverse fill-regions))))
+
 (defun org-fill-paragraph (&optional justify)
   "Fill element at point, when applicable.
 
@@ -22846,21 +22946,7 @@ a footnote definition, try to fill the first paragraph within."
 	       ;; separators, and fill the parts in reverse order to
 	       ;; avoid messing with markers.
 	       (save-excursion
-		 (goto-char end)
-		 (mapc
-		  (lambda (pos)
-		    (fill-region-as-paragraph pos (point) justify)
-		    (goto-char pos))
-		  ;; Find the list of ending positions for line breaks
-		  ;; in the current paragraph.  Add paragraph
-		  ;; beginning to include first slice.
-		  (nreverse
-		   (cons beg
-			 (org-element-map
-			     (org-element--parse-objects
-			      beg end nil (org-element-restriction 'paragraph))
-			     'line-break
-			   (lambda (lb) (org-element-property :end lb)))))))
+		 (org-fill-paragraph-regions beg end justify))
 	       t)))
 	  ;; Contents of `comment-block' type elements should be
 	  ;; filled as plain text, but only if point is within block
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-11 20:44       ` Nicolas Goaziou
@ 2014-08-16  7:50         ` Federico Beffa
  2014-08-16  9:46           ` Nicolas Goaziou
  0 siblings, 1 reply; 16+ messages in thread
From: Federico Beffa @ 2014-08-16  7:50 UTC (permalink / raw)
  To: Rasmus, emacs-orgmode, Nicolas Goaziou

On Mon, Aug 11, 2014 at 10:44 PM, Nicolas Goaziou
<mail@nicolasgoaziou.fr> wrote:
> Rasmus <rasmus@gmx.us> writes:
>
>> How about adding the possibility to add hooks to org-fill-paragraph?
>> So that people can add "extensions" to fill as they want and the core
>> function can rely on org-element only?
>
> You can `defadvice' anything to your heart's content.
>

From the elisp manual:
"...   Advising a function can cause confusion in debugging, since people
who debug calls to the original function may not notice that it has
been modified with advice.  Therefore, if you have the possibility to
change the code of that function to run a hook, please solve the
problem that way.  ..."

Given that people have different opinions on this, a hook would
actually be a good idea. It would give more freedom to the users.
Isn't this the goal of the GNU project :-)

Regards,
Fede

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-16  7:50         ` Federico Beffa
@ 2014-08-16  9:46           ` Nicolas Goaziou
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-16  9:46 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode, Rasmus

Hello,

Federico Beffa <beffa@ieee.org> writes:

> From the elisp manual:
> "...   Advising a function can cause confusion in debugging, since people
> who debug calls to the original function may not notice that it has
> been modified with advice.  Therefore, if you have the possibility to
> change the code of that function to run a hook, please solve the
> problem that way.  ..."
>
> Given that people have different opinions on this, a hook would
> actually be a good idea. It would give more freedom to the users.

Advising is a good user-side mechanism, much more powerful than hooks.
Since it is /user-side/ you should know that it has been modified with
your own advice.

Anyway filling is mode's job, not user's. Again, if you want to take
over this job, use `defadvice', in your config. A hook is pointless, if
not dangerous (the task is not trivial and there are many pitfalls that
could break the structure of the document), here.

No hook in `org-fill-paragraph', please.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line
  2014-08-16  7:38       ` Federico Beffa
@ 2014-08-28 10:10         ` Nicolas Goaziou
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Goaziou @ 2014-08-28 10:10 UTC (permalink / raw)
  To: Federico Beffa; +Cc: emacs-orgmode, Rasmus

Hello,

Federico Beffa <beffa@ieee.org> writes:

> In any case, I've updated my git repository (less than 24h ago) and
> recreated the patch. I've tested it and believe it is working. Why
> don't you give it a try. It's not such a disruptive change.

I looked at the patch. You are still applying it on maint, while the
function in /master/ branch is different. If you look at it, you will
see there are simple ways to achieve what you want (i.e no need for
3 helper functions and 80 additional locs).

Also, I think your implementation is still ill-defined. Consider the
following case:

 Paragraph with a very very ... long line \[some
 math\]

IIUC, M-q should fill the math snippet since it doesn't start nor end
lines. However, if I add text, e.g. additional "very" in the first line,
auto-filling will eventually move "\[some" to the next line, leading to

 Paragraph with a very very very very ... long line
 \[some
 math\]

which cannot be filled since, this time, "\[" starts a line and "\]"
ends one. This math snippet is at the same time fillable and
un-fillable. Schrödinger's snippet. IOW, you also need to prevent
"\[some" to be moved to a new line on its own.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-08-28 10:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-10 13:15 [PATCH] org.el: make org-paragraph-fill ignore \[...\] regions starting and ending a line Federico Beffa
  -- strict thread matches above, loose matches on Subject: below --
2014-08-10 13:13 Federico Beffa
2014-08-11 13:06 ` Nicolas Goaziou
2014-08-11 18:27   ` Federico Beffa
2014-08-11 19:43     ` Rasmus
2014-08-11 20:44       ` Nicolas Goaziou
2014-08-16  7:50         ` Federico Beffa
2014-08-16  9:46           ` Nicolas Goaziou
2014-08-11 20:40     ` Nicolas Goaziou
2014-08-16  7:38       ` Federico Beffa
2014-08-28 10:10         ` Nicolas Goaziou
2014-08-07 13:56 Federico Beffa
2014-08-05 21:45 Federico Beffa
2014-08-09  8:00 ` Nicolas Goaziou
2014-08-09 10:20   ` Federico Beffa
2014-08-09 23:20     ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).