[Prelim. patch] extend org-meta-return to keywords

[Prelim. patch] extend org-meta-return to keywords

[Rasmus]

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

Hi,

Something I have wanted for a while is to have M-RET work "as
expected"(?)  on keyword lines such as #+LATEX_HEADER, #+CAPTION etc.

An "animation" to illustrate, where '|' is point:

    #+CAPTION: this is a |long caption

    #   click <M-RET> ⇒ 

    #+CAPTION: this is a 
    #+CAPTION: |long caption

Would anyone else like this?

Attached is a quick patch that works surprisingly well.  I would work
more on it if you guys agree this would be useful.  It probably need
much more work for corner-cases (any ideas what these are)?

Thanks,
Rasmus

-- 
When the facts change, I change my mind. What do you do, sir?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Add-keyword-support-to-M-RET.patch --]
[-- Type: text/x-diff, Size: 2258 bytes --]

From 60a9cefcc2997cc53a448d1c57ca71935ea6426f Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@gmx.us>
Date: Wed, 19 Nov 2014 15:39:19 +0100
Subject: [PATCH] org.el: Add keyword-support to M-RET

* org.el (org-insert-keyword): New function.
(org-meta-return): May call `org-insert-keyword'.
---
 lisp/org.el | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index dbd2cb7..627c409 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -21286,6 +21286,18 @@ number of stars to add."
 	       (forward-line)))))))
     (unless toggled (message "Cannot toggle heading from here"))))
 
+(defun org-insert-keyword (&optional arg)
+  "Insert a new keyword at point.
+
+ARG may be used to specify a keyword.  Otherwise the keyword is determined from the context.
+
+Mainly used for `org-meta-return'."
+  (interactive "P")
+  (let* ((elm (org-element-at-point))
+	 (key (or arg (and (eq 'keyword (org-element-type elm))
+			   (org-element-property :key elm)))))
+    (and key (insert (format "\n#+%s: " key)))))
+
 (defun org-meta-return (&optional arg)
   "Insert a new heading or wrap a region in a table.
 Calls `org-insert-heading' or `org-table-wrap-region', depending
@@ -21298,12 +21310,13 @@ on context.  See the individual commands for more information."
         (when (eq type 'table-row)
           (setq element (org-element-property :parent element))
           (setq type 'table))
-        (if (and (eq type 'table)
-                 (eq (org-element-property :type element) 'org)
-                 (>= (point) (org-element-property :contents-begin element))
-                 (< (point) (org-element-property :contents-end element)))
-            (call-interactively 'org-table-wrap-region)
-          (call-interactively 'org-insert-heading)))))
+        (cond  ((and (eq type 'table)
+		      (eq (org-element-property :type element) 'org)
+		      (>= (point) (org-element-property :contents-begin element))
+		      (< (point) (org-element-property :contents-end element)))
+		(call-interactively 'org-table-wrap-region))
+	       ((eq type 'keyword) (call-interactively 'org-insert-keyword))
+	       (t (call-interactively 'org-insert-heading))))))
 
 ;;; Menu entries
 
-- 
2.1.3

[patch] extend org-meta-return to keywords

[Rasmus]

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

Hi,

Rasmus <rasmus@gmx.us> writes:

> Something I have wanted for a while is to have M-RET work "as
> expected"(?)  on keyword lines such as #+LATEX_HEADER, #+CAPTION etc.
> [...]
> Attached is a quick patch that works surprisingly well.  I would work
> more on it if you guys agree this would be useful.  It probably need
> much more work for corner-cases (any ideas what these are)?

Attached is a new version of the patch that will respect the variables
that also govern `org-insert-headline'.  It's smarter and preserves the
layout better.

E.g. (where | is the cursor)

   #+CAP|TION: foo

After M-RET it becomes

   #+CAPTION: foo
   #+CAPTION: |

A keyword-line is inserted above when one press M-RET between BOL and "#".

Keywords that an element can only have one of will not be inserted.
E.g.

    #+NAME: foo|

After M-RET it becomes    

    #+NAME: foo
    * |     

Any opinions? 

—Rasmus

-- 
I hear there's rumors on the, uh, Internets. . .

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Add-keyword-support-to-M-RET.patch --]
[-- Type: text/x-diff, Size: 5207 bytes --]

From 1981d0b0a324565659629438ea253cc3e284e0bf Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@gmx.us>
Date: Wed, 19 Nov 2014 15:39:19 +0100
Subject: [PATCH] org.el: Add keyword-support to M-RET

* org.el (org-insert-keyword): New function.
(org-meta-return): May call `org-insert-keyword'.
(org-M-RET-may-split-line): Add keyword.
---
 lisp/org.el | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 11 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 6ab13f4..e8e6b58 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1601,6 +1601,7 @@ contexts.  Valid contexts are:
 headline  when creating a new headline
 item      when creating a new item
 table     in a table field
+keyword   when creating a new keyword
 default   the value to be used for all contexts not explicitly
           customized"
   :group 'org-structure
@@ -1614,6 +1615,7 @@ default   the value to be used for all contexts not explicitly
 			   (const headline)
 			   (const item)
 			   (const table)
+			   (const keyword)
 			   (const default))
 		   (boolean)))))
 
@@ -7725,8 +7727,8 @@ split the line and create a new headline with the text in the
 current line after point \(see `org-M-RET-may-split-line' on how
 to modify this behavior).
 
-If point is at the beginning of a normal line, turn this line
-into a heading.
+If point is at the beginning of a normal line, excluding some
+keywords, turn this line into a heading.
 
 When INVISIBLE-OK is set, stop at invisible headlines when going
 back.  This is important for non-interactive uses of the
@@ -21291,10 +21293,57 @@ number of stars to add."
 	       (forward-line)))))))
     (unless toggled (message "Cannot toggle heading from here"))))
 
+(defun org-insert-keyword (&optional arg)
+  "Insert a new keyword-line.
+
+If point is between the beginning of the line and the start of
+the keyword, a keyword-line is inserted above the current one.
+
+If point is in the middle of a keyword-line, split the line
+depending on the value of `org-M-RET-may-split-line'.  See the
+docstring of this variable for further details.
+
+If point is within the keyword a new keyword-line is inserted
+below.
+
+Currently arg is ignored and the keyword is determined from the
+context.
+
+The function is used by `org-meta-return'.  Note that keywords
+that can only occur once per element are ignored (through
+`org-meta-return')."
+  (interactive "P")
+  (let* ((may-split (org-get-alist-option
+		     org-M-RET-may-split-line 'keyword))
+	 (point-at-bol-p (looking-back "^[[:space:]]*"))
+	 (end-of-keyword
+	  (save-excursion
+	    (beginning-of-line)
+	    (search-forward-regexp
+	     org-element--affiliated-re (point-at-eol) t)
+	    (point)))
+	 (elm (org-element-at-point))
+	 (key (and (eq 'keyword (org-element-type elm))
+		   (org-element-property :key elm))))
+    (when key
+      (when point-at-bol-p (open-line 1)
+	    ;; Open-line makes sometimes ruins indention of the
+	    ;; previous line.
+	    (save-excursion (forward-line 1)
+			    (org-indent-line)))
+      (unless may-split (end-of-line))
+      (unless point-at-bol-p
+	(when (< (point) end-of-keyword)
+	  (goto-char end-of-keyword))
+	(insert "\n"))
+      (insert (format "#+%s: " key))
+      (org-indent-line))))
+
 (defun org-meta-return (&optional arg)
-  "Insert a new heading or wrap a region in a table.
-Calls `org-insert-heading' or `org-table-wrap-region', depending
-on context.  See the individual commands for more information."
+  "Insert a new heading, a new keyword or wrap a region in a table.
+Calls `org-insert-heading', `org-insert-keyword' or
+`org-table-wrap-region', depending on context.  See the
+individual commands for more information."
   (interactive "P")
   (org-check-before-invisible-edit 'insert)
   (or (run-hook-with-args-until-success 'org-metareturn-hook)
@@ -21303,12 +21352,20 @@ on context.  See the individual commands for more information."
         (when (eq type 'table-row)
           (setq element (org-element-property :parent element))
           (setq type 'table))
-        (if (and (eq type 'table)
-                 (eq (org-element-property :type element) 'org)
-                 (>= (point) (org-element-property :contents-begin element))
-                 (< (point) (org-element-property :contents-end element)))
-            (call-interactively 'org-table-wrap-region)
-          (call-interactively 'org-insert-heading)))))
+        (cond  ((and (eq type 'table)
+		      (eq (org-element-property :type element) 'org)
+		      (>= (point) (org-element-property :contents-begin element))
+		      (< (point) (org-element-property :contents-end element)))
+		(call-interactively 'org-table-wrap-region))
+	       ((and (eq type 'keyword)
+		     ;; Keyword such as LATEX, ATTR_LATEX, CAPTION, and HEADER,
+		     ;; LATEX_HEADER, LATEX etc. can occur multiple times.
+		     (let ((key (org-element-property :key element)))
+		       (if (member key org-element-affiliated-keywords)
+			   (member key org-element-multiple-keywords)
+			 t)))
+		(call-interactively 'org-insert-keyword))
+	       (t (call-interactively 'org-insert-heading))))))
 
 ;;; Menu entries
 
-- 
2.1.3

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Hello,

Rasmus <rasmus@gmx.us> writes:

> Attached is a new version of the patch that will respect the variables
> that also govern `org-insert-headline'.  It's smarter and preserves the
> layout better.

Thanks for the patch. However there are already mechanisms to complete
keywords faster (e.g., M-TAB, yasnippet). Of course, yours is more
efficient, but also very specific. It makes sense on very few keywords.
I have no strong opinion here, but I don't think it is general enough to
go into core.

Moreover, it can get in the way of expected M-RET behaviour, as in the
following example

  - item

  | #+caption: test
    untenrsiu

where M-RET is expected to insert an item.

Anyway, some comments follow.

> +	 (point-at-bol-p (looking-back "^[[:space:]]*"))

  (point-at-bol-p (save-excursion (skip-chars-backward " \t") (bolp)))

See last paragraph in `looking-back' docstring.

> +	 (end-of-keyword
> +	  (save-excursion
> +	    (beginning-of-line)
> +	    (search-forward-regexp

Nitpick: `re-search-forward'

> +	     org-element--affiliated-re (point-at-eol) t)

Nitpick: `line-end-position'

Also, you're not supposed to use `org-element--affiliated-re', as
suggested by the double hyphen. If you want to tell when point is at an
affiliated keyword, use

  (< (point) (org-element-property :post-affiliated element))

where ELEMENT is returned by `org-element-at-point'. You can then
extract its name with, e.g.,

  (save-excursion 
   (beginning-of-line)
   (looking-at "#\\+\\(.+?\\):")
   (upcase (org-match-string-no-properties 1)))
 
> +	 (elm (org-element-at-point))
> +	 (key (and (eq 'keyword (org-element-type elm))
> +		   (org-element-property :key elm))))
> +    (when key

KEY is nil when at an affiliated keyword. So this function is a no-op
on #+CAPTION: and alike.  Try it on a real caption, e.g.,

  #+CAPTION: caption
  Paragraph

> +      (when point-at-bol-p (open-line 1)
> +	    ;; Open-line makes sometimes ruins indention of the
> +	    ;; previous line.
> +	    (save-excursion (forward-line 1)
> +			    (org-indent-line)))

Don't use `open-line' at all. Insert "\n" where appropriate. Also avoid
using `org-indent-line' since you can already know what the expected
indentation is (i.e., by storing it earlier).

> +      (unless may-split (end-of-line))
> +      (unless point-at-bol-p
> +	(when (< (point) end-of-keyword)
> +	  (goto-char end-of-keyword))
> +	(insert "\n"))
> +      (insert (format "#+%s: " key))
> +      (org-indent-line))))

Ditto. Use stored indentation.

> +	       ((and (eq type 'keyword)
> +		     ;; Keyword such as LATEX, ATTR_LATEX, CAPTION, and HEADER,
> +		     ;; LATEX_HEADER, LATEX etc. can occur multiple times.
> +		     (let ((key (org-element-property :key element)))
> +		       (if (member key org-element-affiliated-keywords)
> +			   (member key org-element-multiple-keywords)
> +			 t)))

KEY cannot belong to `org-element-affiliated-keywords'.  See above.

Also,

  (or (not (member key org-element-affiliated-keywords))
      (member key org-element-multiple-keywords))


Regards,

-- 
Nicolas Goaziou

Re: [patch] extend org-meta-return to keywords

[Rasmus Pank Roulund]

Hi,

Thanks for the comments.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>> Attached is a new version of the patch that will respect the variables
>> that also govern `org-insert-headline'.  It's smarter and preserves the
>> layout better.
>
> Thanks for the patch. However there are already mechanisms to complete
> keywords faster (e.g., M-TAB, yasnippet).

Personally, I never figured out how to use yasnippet.  M-TAB is for
completion of new lines, not for continuation of ongoing work.

> Of course, yours is more efficient, but also very specific.

So say you do
#+LATEX_HEADER: \usepackage{biblatex}
Now you need to add a bib resource.  It's nice to quickly be able to get
a new #+LATEX_HEADER-line.  It's specific, but it's meant as a
continuation of the M-RET logic.

> It makes sense on very few keywords.  I have no strong opinion here,
> but I don't think it is general enough to go into core.

OK.  Hopefully some more people will chime in so we can make an educated
guess.

> Moreover, it can get in the way of expected M-RET behaviour, as in the
> following example
>
>   - item
>
>   | #+caption: test
>     untenrsiu
> 
> where M-RET is expected to insert an item.

Sure. . .  I'm not sure expected behavior is what I'd like here.  But
that's the initial point, I guess.

> Anyway, some comments follow.

Thanks for those.

Cheers,
Rasmus

-- 
The second rule of Fight Club is: You do not talk about Fight Club

Re: [patch] extend org-meta-return to keywords

[Rasmus]

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

Hi,

First, Thierry, I think this patch fixes the second misbehavior you
observed.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:


I looked through your comments and as always they made the patch much
better!  Thanks!

> Moreover, it can get in the way of expected M-RET behaviour, as in the
> following example
>
>   - item
>
>   | #+caption: test
>     untenrsiu

I don't know if I should do something about this case.  I guess it would
be possible, but I find it awkward when behavior $BUTTON depends not
only on not only the adjacent element, but also the previous element.

If it's important, I guess it should be toggled explicitly on by a
custom variable.

>   (point-at-bol-p (save-excursion (skip-chars-backward " \t") (bolp)))
>
> See last paragraph in `looking-back' docstring.

Thanks, that's nice.

> Also, you're not supposed to use `org-element--affiliated-re', as
> suggested by the double hyphen. If you want to tell when point is at an
> affiliated keyword, use
>
>   (< (point) (org-element-property :post-affiliated element))

That's a great tip in its own right, but actually I used
`org-element--affiliated-re' for determining if point was "in"/"on" the
keyword, as here:

       #+CAPT|ION: foo

Anyway, I changed it to fixed regexp.  Presently, it's hardcoded.
Should I introduce a new defvar or just live with the hardcodedness?

> where ELEMENT is returned by `org-element-at-point'. You can then
> extract its name with, e.g.,
>
>   (save-excursion 
>    (beginning-of-line)
>    (looking-at "#\\+\\(.+?\\):")
>    (upcase (org-match-string-no-properties 1)))

In the current patch I run this code twice, once in `org-meta-return',
once in `org-insert-keyword' (basically twice).  I would probably be
possible to pass the keyword as an argument, but I'm not sure it's worth
it.  Also, `org-insert-keyword' is not really using org-element anymore.

>> +	 (elm (org-element-at-point))
>> +	 (key (and (eq 'keyword (org-element-type elm))
>> +		   (org-element-property :key elm))))
>> +    (when key
>
> KEY is nil when at an affiliated keyword. So this function is a no-op
> on #+CAPTION: and alike.  Try it on a real caption, e.g.,
>
>   #+CAPTION: caption
>   Paragraph

Good call.  I detect this case with
(org-element-property :post-affiliated element) now.  It seems to work
in the variation of this I could come up with.  Quick testing suggest
that affiliated keywords are always on the top of an (greater) element.

>> +      (when point-at-bol-p (open-line 1)
>> +	    ;; Open-line makes sometimes ruins indention of the
>> +	    ;; previous line.
>> +	    (save-excursion (forward-line 1)
>> +			    (org-indent-line)))
>
> Don't use `open-line' at all. Insert "\n" where appropriate. Also avoid
> using `org-indent-line' since you can already know what the expected
> indentation is (i.e., by storing it earlier).

Thanks.  Actually, I think I saw `open-line' in `org-insert-headline`
and that's why I used it.  Perhaps that's another patch.
 
>> +	       ((and (eq type 'keyword)
>> +		     ;; Keyword such as LATEX, ATTR_LATEX, CAPTION, and HEADER,
>> +		     ;; LATEX_HEADER, LATEX etc. can occur multiple times.
>> +		     (let ((key (org-element-property :key element)))
>> +		       (if (member key org-element-affiliated-keywords)
>> +			   (member key org-element-multiple-keywords)
>> +			 t)))
>
> KEY cannot belong to `org-element-affiliated-keywords'.  See above.

It test that *if* it's a member of `org-element-affiliated-keywords'
then it should also be a member of `org-element-multiple-keywords'.
Anyway, your test is nicer:

> Also,
>
>   (or (not (member key org-element-affiliated-keywords))
>       (member key org-element-multiple-keywords))

Cheers,
Rasmus

-- 
Summon the Mothership!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Add-keyword-support-to-M-RET.patch --]
[-- Type: text/x-diff, Size: 5698 bytes --]

From 2e70cf7c7df61eebcc10e48077c5212d9015900e Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@gmx.us>
Date: Wed, 19 Nov 2014 15:39:19 +0100
Subject: [PATCH] org.el: Add keyword-support to M-RET

* org.el (org-insert-keyword): New function.
(org-meta-return): May call `org-insert-keyword'.
(org-M-RET-may-split-line): Add keyword.
---
 lisp/org.el | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 80 insertions(+), 11 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 6ab13f4..3405ab4 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1601,6 +1601,7 @@ contexts.  Valid contexts are:
 headline  when creating a new headline
 item      when creating a new item
 table     in a table field
+keyword   when creating a new keyword
 default   the value to be used for all contexts not explicitly
           customized"
   :group 'org-structure
@@ -1614,6 +1615,7 @@ default   the value to be used for all contexts not explicitly
 			   (const headline)
 			   (const item)
 			   (const table)
+			   (const keyword)
 			   (const default))
 		   (boolean)))))
 
@@ -7725,8 +7727,8 @@ split the line and create a new headline with the text in the
 current line after point \(see `org-M-RET-may-split-line' on how
 to modify this behavior).
 
-If point is at the beginning of a normal line, turn this line
-into a heading.
+If point is at the beginning of a normal line, excluding some
+keywords, turn this line into a heading.
 
 When INVISIBLE-OK is set, stop at invisible headlines when going
 back.  This is important for non-interactive uses of the
@@ -21291,10 +21293,65 @@ number of stars to add."
 	       (forward-line)))))))
     (unless toggled (message "Cannot toggle heading from here"))))
 
+(defun org-insert-keyword (&optional arg)
+  "Insert a new keyword-line, such as #+CAPTION or #+LATEX_HEADER.
+
+If point is between the beginning of the line and the beginning
+of the keyword, as denoted by \"#\", a keyword-line is inserted
+above the current one.  If the point is within the keyword, a new
+keyword-line is inserted below.
+
+If point is in the middle of a keyword-line, after the keyword
+itself, split the line depending on the value of
+`org-M-RET-may-split-line'.  See the docstring of this variable
+for further details.
+
+Currently arg is ignored and the keyword is determined from the
+context.
+
+The function is used by `org-meta-return'.  Note, affiliated
+keywords that are not allowed to appear multiple times are
+ignored by `org-meta-return' (see
+`org-element-affiliated-keywords' and
+`org-element-multiple-keywords')."
+  (interactive "P")
+  (let* ((may-split (org-get-alist-option
+		     org-M-RET-may-split-line 'keyword))
+	 (point-at-bol-p
+	  (save-excursion (skip-chars-backward " \t") (bolp)))
+	 (indention
+	  (buffer-substring
+	   (line-beginning-position)
+	   (save-excursion
+	     (beginning-of-line)
+	     (skip-chars-forward " \t")
+	     (point))))
+	 (keyword-re "#\\+\\(.+?\\):")
+	 (key (save-excursion (beginning-of-line)
+			      (skip-chars-forward " \t")
+			      (looking-at keyword-re)
+			      (upcase (org-match-string-no-properties 1))))
+	 (end-of-keyword (save-excursion
+			   (beginning-of-line)
+			   (re-search-forward keyword-re (line-end-position) t)
+			   (point))))
+    (when key
+      (when point-at-bol-p
+	(save-excursion
+	  (beginning-of-line)
+	  (insert "\n")))
+      (unless may-split (end-of-line))
+      (unless point-at-bol-p
+	(when (< (point) end-of-keyword) (end-of-line))
+	(insert "\n"))
+      (insert (concat indention (format "#+%s: " key)))
+      (delete-region (point) (save-excursion (skip-chars-forward " \t") (point))))))
+
 (defun org-meta-return (&optional arg)
-  "Insert a new heading or wrap a region in a table.
-Calls `org-insert-heading' or `org-table-wrap-region', depending
-on context.  See the individual commands for more information."
+  "Insert a new heading, a new keyword or wrap a region in a table.
+Calls `org-insert-heading', `org-insert-keyword' or
+`org-table-wrap-region', depending on context.  See the
+individual commands for more information."
   (interactive "P")
   (org-check-before-invisible-edit 'insert)
   (or (run-hook-with-args-until-success 'org-metareturn-hook)
@@ -21303,12 +21360,24 @@ on context.  See the individual commands for more information."
         (when (eq type 'table-row)
           (setq element (org-element-property :parent element))
           (setq type 'table))
-        (if (and (eq type 'table)
-                 (eq (org-element-property :type element) 'org)
-                 (>= (point) (org-element-property :contents-begin element))
-                 (< (point) (org-element-property :contents-end element)))
-            (call-interactively 'org-table-wrap-region)
-          (call-interactively 'org-insert-heading)))))
+        (cond  ((and (eq type 'table)
+		      (eq (org-element-property :type element) 'org)
+		      (>= (point) (org-element-property :contents-begin element))
+		      (< (point) (org-element-property :contents-end element)))
+		(call-interactively 'org-table-wrap-region))
+	       ((and
+		 (or (eq type 'keyword)
+		     (<  (point) (org-element-property :post-affiliated element)))
+		 (let ((key (save-excursion
+			      (beginning-of-line)
+			      (skip-chars-forward " \t")
+			      (and (looking-at "#\\+\\(.+?\\):")
+				   (upcase (org-match-string-no-properties 1))))))
+		   (and key
+			(or (not (member key org-element-affiliated-keywords))
+			    (member key org-element-multiple-keywords)))))
+		(call-interactively 'org-insert-keyword))
+	       (t (call-interactively 'org-insert-heading))))))
 
 ;;; Menu entries
 
-- 
2.1.3

Re: [patch] extend org-meta-return to keywords

[Thierry Banel]

Le 22/11/2014 20:26, Rasmus a écrit :
> Hi,
>
> First, Thierry, I think this patch fixes the second misbehavior you
> observed.

Fixed!

I was wondering, is there any reason why the new inserted keyword is
UPPERCASED?
If not, maybe it would be better to keep the oriGiNaL cASe.

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

>> Moreover, it can get in the way of expected M-RET behaviour, as in the
>> following example
>>
>>   - item
>>
>>   | #+caption: test
>>     untenrsiu
>
> I don't know if I should do something about this case.  I guess it would
> be possible, but I find it awkward when behavior $BUTTON depends not
> only on not only the adjacent element, but also the previous element.
>
> If it's important, I guess it should be toggled explicitly on by a
> custom variable.

M-RET, is, first and foremost, an important keybinding for editing the
/structure/ of the document. The behaviour you want to add has nothing
to do with structure.

As a consequence, I'm not sure it should go with M-RET, and if it does,
I'm pretty sure it should not override the main purpose of the binding.
Inserting headlines, and possibly items, is much more important than
duplicating keywords.

>> Also, you're not supposed to use `org-element--affiliated-re', as
>> suggested by the double hyphen. If you want to tell when point is at an
>> affiliated keyword, use
>>
>>   (< (point) (org-element-property :post-affiliated element))
>
> That's a great tip in its own right, but actually I used
> `org-element--affiliated-re' for determining if point was "in"/"on" the
> keyword, as here:
>
>        #+CAPT|ION: foo

You need to check if either element has `keyword' type or if you're on
an affiliated keyword. If you ignore this, like you did, you end up
introducing false positives like

  #+begin: |something
  ...
  #+end:

> Anyway, I changed it to fixed regexp.  Presently, it's hardcoded.
> Should I introduce a new defvar or just live with the hardcodedness?

A defconst is preferable, IMO.

> Also, `org-insert-keyword' is not really using org-element anymore.

It should since you make it interactive and can, therefore, be called on
its own.

>>> +	       ((and (eq type 'keyword)
>>> +		     ;; Keyword such as LATEX, ATTR_LATEX, CAPTION, and HEADER,
>>> +		     ;; LATEX_HEADER, LATEX etc. can occur multiple times.
>>> +		     (let ((key (org-element-property :key element)))
>>> +		       (if (member key org-element-affiliated-keywords)
>>> +			   (member key org-element-multiple-keywords)
>>> +			 t)))
>>
>> KEY cannot belong to `org-element-affiliated-keywords'.  See above.
>
> It test that *if* it's a member of `org-element-affiliated-keywords'
> then it should also be a member of `org-element-multiple-keywords'.

I was pointing out that your test was always false. Anyway, it doesn't
matter anymore since you changed that part.

Some comments follow:

> +	 (indention
> +	  (buffer-substring
> +	   (line-beginning-position)
> +	   (save-excursion
> +	     (beginning-of-line)
> +	     (skip-chars-forward " \t")
> +	     (point))))

Another option:

  (ind (org-get-indentation))

Then, after inserting "\n",

  (org-indent-line-to ind)

> +	 (keyword-re "#\\+\\(.+?\\):")
> +	 (key (save-excursion (beginning-of-line)
> +			      (skip-chars-forward " \t")
> +			      (looking-at keyword-re)
> +			      (upcase (org-match-string-no-properties 1))))

As discussed above, this is too fragile.  You need to duplicate the
checks done in `org-meta-return' or refactor the code.

> +	 (end-of-keyword (save-excursion
> +			   (beginning-of-line)
> +			   (re-search-forward keyword-re (line-end-position) t)
> +			   (point))))

  (end-of-keyword (save-excursion (beginning-of-line) (search-forward ":"))

> +    (when key

I think end-of-keyword should be bound after KEY is checked to avoid
errors.

> +	(insert "\n"))
> +      (insert (concat indention (format "#+%s: " key)))

  (insert indentation (format "#+%s: " key))

> +	       ((and
> +		 (or (eq type 'keyword)
> +		     (<  (point) (org-element-property :post-affiliated element)))
> +		 (let ((key (save-excursion
> +			      (beginning-of-line)
> +			      (skip-chars-forward " \t")
> +			      (and (looking-at "#\\+\\(.+?\\):")

  "^[ \t]*#\\+\\(.+?\\):"

avoids the `skip-chars-forward' part.

> +				   (upcase (org-match-string-no-properties 1))))))

Actually, you don't need `upcase' if you use `member-ignore-case' below.

> +		   (and key
> +			(or (not (member key org-element-affiliated-keywords))
> +			    (member key org-element-multiple-keywords)))))

See above.


Regards,

Re: [patch] extend org-meta-return to keywords

[Thierry Banel]

Le 23/11/2014 00:20, Nicolas Goaziou a écrit :
> M-RET, is, first and foremost, an important keybinding for editing the
> /structure/ of the document. The behaviour you want to add has nothing
> to do with structure.

I guess this is the main decision point about Rasmus's proposal.

I'm not sure I understand the /structure/ argument. (Is structure about
hiding blocks? Parsing org docs?).

Nicolas, you have worked a lot on this topic (org-element, new drawer
syntax). May be you could elaborate on this.

> As a consequence, I'm not sure it should go with M-RET, and if it does,
> I'm pretty sure it should not override the main purpose of the binding.
> Inserting headlines, and possibly items, is much more important than
> duplicating keywords.
>

We already have M-RET for splitting a table cell
(org-table-wrap-region). Should this feature be remapped to some other
key-binding?

Regards,
Thierry

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Hello,

Thierry Banel <tbanelwebmin@free.fr> writes:

> I'm not sure I understand the /structure/ argument. (Is structure
> about hiding blocks? Parsing org docs?).
>
> Nicolas, you have worked a lot on this topic (org-element, new drawer
>syntax). May be you could elaborate on this.

M-RET, C-RET are the major mean to insert headlines (the structure of
the document) and items, which are very important features. I'm pretty
sure these bindings are in muscle memory of many Org users. Asking them
to take into account context before using M-RET is a big downside.
I think, on the contrary, that it should do its job without exception.

> We already have M-RET for splitting a table cell
> (org-table-wrap-region). Should this feature be remapped to some other
> key-binding?

Binding `org-table-wrap-region' to an important keybinding such as M-RET
is wrong IMO. Anyway, that's not the subject here, which is to avoid
adding more clutter to M-RET. Let's not make it another C-c C-c.


Regards,

-- 
Nicolas Goaziou

Re: [patch] extend org-meta-return to keywords

[Rasmus]

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Thierry Banel <tbanelwebmin@free.fr> writes:
>
>> I'm not sure I understand the /structure/ argument. (Is structure
>> about hiding blocks? Parsing org docs?).
>>
>> Nicolas, you have worked a lot on this topic (org-element, new drawer
>>syntax). May be you could elaborate on this.
>
> M-RET, C-RET are the major mean to insert headlines (the structure of
> the document) and items, which are very important features. I'm pretty
> sure these bindings are in muscle memory of many Org users. Asking them
> to take into account context before using M-RET is a big downside.
> I think, on the contrary, that it should do its job without exception.

I don't find it complicated at all. . .  It's DWIM!

>> We already have M-RET for splitting a table cell
>> (org-table-wrap-region). Should this feature be remapped to some other
>> key-binding?
>
> Binding `org-table-wrap-region' to an important keybinding such as M-RET
> is wrong IMO. Anyway, that's not the subject here, which is to avoid
> adding more clutter to M-RET. Let's not make it another C-c C-c.

The fact that `org-table-wrap-region' is bound to M-RET is the reason
I'd bind `org-insert-keyword' to M-RET. . .

Anyway, do we have /another/ key that these two functions could be
migrated to?

Cheers,
Rasmus

-- 
And I faced endless streams of vendor-approved Ikea furniture. . .

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> I don't find it complicated at all. . .  It's DWIM!

I understand your DWIM argument. But we're talking about one of the most
central keybindings in Org, much like C-c C-c. Except I do not consider
C-c C-c as a usability model. Also, C-c C-c is not really dedicated to
headlines.

> The fact that `org-table-wrap-region' is bound to M-RET is the reason
> I'd bind `org-insert-keyword' to M-RET. . .

I really don't understand the reason behind binding
`org-table-wrap-region' (how often do you use that?) on M-RET.

> Anyway, do we have /another/ key that these two functions could be
> migrated to?

There is `org-table-copy-down' bound to S-RET. The idea is similar.


Regards,

Re: [patch] extend org-meta-return to keywords

[Rasmus]

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> I—obviously—think what I propose is better than what we have now.  Let's
>> go through the current functionality.
>
> In this case I value simplicity over complexity (or "smartness",
> whatever you call it). Anyone, including newcomers, should be able to
> use M-RET without knowing about (affiliated) keywords, or be bothered
> with them.

"Newcomers" ain't stupid.  My guess is it would feel very intuitive to
be able to repeat e.g. LATEX_HEADINGs.

> Also, headlines are first-class citizens in Org. Keybindings acting on
> them shouldn't be bothered with other citizens.

But "* #+LATEX_HEADER: \usepackage{foo}" or whatever is possibly the
most useless headline in the history of Org. . .

>> I don't find it complicated at all. . .  It's DWIM!
>
> I understand your DWIM argument. But we're talking about one of the most
> central keybindings in Org, much like C-c C-c. Except I do not consider
> C-c C-c as a usability model. Also, C-c C-c is not really dedicated to
> headlines.

I guess I mainly use C-c C-c for tags.  I don't know what else it's used
for (without looking at the code). . .

>> The fact that `org-table-wrap-region' is bound to M-RET is the reason
>> I'd bind `org-insert-keyword' to M-RET. . .
>
> I really don't understand the reason behind binding
> `org-table-wrap-region'

> (how often do you use that?) on M-RET.

Very rarely.  I obviously use M-RET all the time for items and
headlines.  `org-table-wrap-region' seems like a solution looking for a
problem.

>> Anyway, do we have /another/ key that these two functions could be
>> migrated to?
>
> There is `org-table-copy-down' bound to S-RET. The idea is similar.

That seems less crowded.  In fact it doesn't work outside of tables ATM.

Still, I think overload M-RET is more intuitive, but I fear our opinions
will fail to converge.

—Rasmus

-- 
Lasciate ogni speranza o voi che entrate: siete nella mani di'machellaio

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> "Newcomers" ain't stupid.

I never said that.

However, here is an historical data point. SCHEDULED, DEADLINE and
CLOSED keywords, aka planning info, are not included as properties in
properties drawer so newcomers do not encounter invisible contents for
such basic tasks.

This is the same logic here.

> My guess is it would feel very intuitive to be able to repeat e.g.
> LATEX_HEADINGs.

M-RET is a good fit for almost anything. That's all the problem.

As far as UI goes, I think we should prevent some keybindings from being
context sensitive. This is particularly true for those related to core
features (outliner, agenda, TODO list). Why ? Because basic Org features
should ideally be very simple to use.

Org is mostly geared towards "power users". The feature you're
implementing is also for "power users". However, /any/ user should be
able to master the core features in 5 minutes.

> But "* #+LATEX_HEADER: \usepackage{foo}" or whatever is possibly the
> most useless headline in the history of Org. . .

Who knows? It depends on the context, on the next commands (e.g. C-o)...

Hitting "B" key at

  #+|LATEX_HEADER: \usepackage{foo}

is as useless. Nevertheless, I suppose you don't want to bind "B" key to
something else than `org-self-insert-command' here.

> Still, I think overload M-RET is more intuitive,

"Intuitiveness" is in the eye of the beholder... Maybe some users will
not find intuitive that M-RET usually inserts a headline... unless it is
in a plain list, in which case it inserts an item... oh wait, if it is
at a keyword it can duplicate it... but, wait, it doesn't happen for
every keyword... ah! and in source blocks it...

> but I fear our opinions will fail to converge.

If you want to convince me, show me that this binding makes Org simpler
to use, or, at least, equally simple. Until then, I stand on my ground.


Regards,

Re: [patch] extend org-meta-return to keywords

[Rasmus]

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> Still, I think overload M-RET is more intuitive,
>
> "Intuitiveness" is in the eye of the beholder... 

Clearly.

> Maybe some users will not find intuitive that M-RET usually inserts a
> headline... unless it is in a plain list, in which case it inserts an
> item... oh wait, if it is at a keyword it can duplicate it... but,
> wait, it doesn't happen for every keyword... ah! and in source blocks
> it...

Not to forget tables. . .

>> but I fear our opinions will fail to converge.
>
> If you want to convince me, show me that this binding makes Org simpler
> to use, or, at least, equally simple. Until then, I stand on my ground.

This is a Sisyphean task.  Even if a number of participants of this ML
would chime in, we would still not know whether it would confuse or
benefit new users.

From my point of view, we can make every function tied to M-RET beyond
`org-insert-headline' configurable and turned off by default.  This may,
however, also add confusion ("why did M-RET work in X's Org but not in
mine...").

At this point, I doubt I can add novel, useful inputs to the discussion.

Cheers,
Rasmus

-- 
And I faced endless streams of vendor-approved Ikea furniture. . .

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> From my point of view, we can make every function tied to M-RET beyond
> `org-insert-headline' configurable and turned off by default.  This may,
> however, also add confusion ("why did M-RET work in X's Org but not in
> mine...").

In this case, S-RET is a superior (as in less confusing) choice.

Anyway, I assume M-RET on keywords is just a first step. So, what's the
big picture? I think it would help to know the complete specifications
of the M-RET you envision, as it could make more sense than the sum of
its parts.


Regards,

M-RET vs C-RET

[Sebastien Vauban]

Hello,

In a similar vein to this thread, I've always wondered if the following
is normal or not.

Having such a file:

--8<---------------cut here---------------start------------->8---
** TODO Research

- Who are the competitors?
- What are our product's advantages?
- Target market
- Elevator pitch|

Check out with Laura...

Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

** TODO Define priorities
--8<---------------cut here---------------end--------------->8---

If I want to cut the task into 2 at the point of the cursor (vertical
bar, after "pitch"), I thought doing M-RET or C-RET were the way to go.

But:

- M-RET inserts a new item in the list
- C-RET inserts a new headline... but after the Lorem paragraph.

That's only a minor annoyance to me. I often just undo, and type "**"
myself.

If you think that the above is expected, just forget about this.

Or, maybe, I do have to (un)set some variable?

Best regards,
  Seb

-- 
Sebastien Vauban

Re: M-RET vs C-RET

[Nicolas Goaziou]

Hello,

Sebastien Vauban <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> In a similar vein to this thread, I've always wondered if the following
> is normal or not.
>
> Having such a file:
>
> ** TODO Research
>
> - Who are the competitors?
> - What are our product's advantages?
> - Target market
> - Elevator pitch|
>
> Check out with Laura...
>
> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
> tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
> quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
> consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
> cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
> proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
>
> ** TODO Define priorities
>
> If I want to cut the task into 2 at the point of the cursor (vertical
> bar, after "pitch"), I thought doing M-RET or C-RET were the way to go.
>
> But:
>
> - M-RET inserts a new item in the list
> - C-RET inserts a new headline... but after the Lorem paragraph.

One used to be able to type "C-u M-RET" and get a headline in the middle
of the list, but this was changed in
6c9b3ad91f6d3758471d194b75cbbf7a1030e18b.


Regards,

-- 
Nicolas Goaziou

Re: M-RET vs C-RET

[Rasmus]

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>> From my point of view, we can make every function tied to M-RET beyond
>> `org-insert-headline' configurable and turned off by default.  This may,
>> however, also add confusion ("why did M-RET work in X's Org but not in
>> mine...").
>
> In this case, S-RET is a superior (as in less confusing) choice.

I see.  That's probably OK.

> Anyway, I assume M-RET on keywords is just a first step. So, what's the
> big picture? I think it would help to know the complete specifications
> of the M-RET you envision, as it could make more sense than the sum of
> its parts.

You're the architect.  I just submit an occasional patch or bug-fix
(under much guidance).

I don't have a grand vision, but, ideally, I'd want M-RET to "do the
right thing", which is my book is often create an element similar to
element at point, and is certainly not but my #+begin_src emacs-lisp
code on a headline.  I agree the logical action is to the eye of the
beholder.  To me, some elements have a very clear-cut "next logical
thing" (item, headline, white space (headline), some keywords, maybe
tables), others don't (e.g. src-blocks and export blocks.).  IMO, we can
disable most of element-actions (literately keywords and tables) out of
the box, much like e.g. `scroll-left'.

Sebastien Vauban <sva-news@mygooglest.com>
writes:

> Having such a file:
>
> ** TODO Research
> - [...]
> - Elevator pitch|
>
> Check out with Laura...
>
> Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
> [...]
> 
> If I want to cut the task into 2 at the point of the cursor (vertical
> bar, after "pitch"), I thought doing M-RET or C-RET were the way to go.

Here's another of my pet-griefs


- a
- b

| → M-RET will give me an itme 
| → M-RET will give me a headline

Why is the behavior a function of amount of whitespace/newlines to
nearest element?  This makes not sense to me and goes against what I
want, namely act in accordance to element at point. . .

—Rasmus

-- 
Need more coffee. . .

Re: M-RET vs C-RET

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> I don't have a grand vision, but, ideally, I'd want M-RET to "do the
> right thing", which is my book is often create an element similar to
> element at point, and is certainly not but my #+begin_src emacs-lisp
> code on a headline.  I agree the logical action is to the eye of the
> beholder.  To me, some elements have a very clear-cut "next logical
> thing" (item, headline, white space (headline), some keywords, maybe
> tables), others don't (e.g. src-blocks and export blocks.).  IMO, we can
> disable most of element-actions (literately keywords and tables) out of
> the box, much like e.g. `scroll-left'.

It would be nice to have complete specifications of "do the right
thing".

Also, it is important to have a way to insert a headline whatever is
around, and one to insert a headline at the end of the current section
or even great-parent.

Currently C-RET is sub-optimal since it is equivalent to C-u M-RET. It
might be possible to re-define both M-RET and C-RET so they can cover
all use-cases in a predictable, and meaningful, fashion.

> Here's another of my pet-griefs
> - a
> - b
>
> | → M-RET will give me an itme 
> | → M-RET will give me a headline
>
> Why is the behavior a function of amount of whitespace/newlines to
> nearest element?  This makes not sense to me and goes against what I
> want, namely act in accordance to element at point. . .

Blank lines belong to the element at point above.

In particular, number of blank lines is meaningful in plain lists and
footnote definitions (2 blank lines mark the end of the element). In the
first line, you're still in the list, in the next one, you're not
anymore, hence the behaviour.

Think about

  - a

  - b

  |


Regards,

Re: M-RET vs C-RET

[Rasmus]

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>> I don't have a grand vision, but, ideally, I'd want M-RET to "do the
>> right thing", which is my book is often create an element similar to
>> element at point, and is certainly not but my #+begin_src emacs-lisp
>> code on a headline.  I agree the logical action is to the eye of the
>> beholder.  To me, some elements have a very clear-cut "next logical
>> thing" (item, headline, white space (headline), some keywords, maybe
>> tables), others don't (e.g. src-blocks and export blocks.).  IMO, we can
>> disable most of element-actions (literately keywords and tables) out of
>> the box, much like e.g. `scroll-left'.
>
> It would be nice to have complete specifications of "do the right
> thing".

I agree.

Some quick thoughts:

babel-call → maybe insert new call line?
comment → new commented line?
Drawer, property-drawer → insert new drawer template?
fixed-width → clone :
headline → `org-insert-headline'
inlinetask → insert new inlinetask? 
item → `org-insert-headline'
keyword → `org-insert-keyword' but doesn't cover all keywords...
paragraph → `org-insert-headline'
plain-list → `org-insert-headline'
table-row → what it does now

No clue:

center-block → 
clock →
comment-block → 
diary-sexp →
dynamic-block → 
example-block → 
export-block → 
horizontal-rule →
latex-environment →
node-property →
planning → 
quote-block → 
special-block →
src-block →
table →
verse-block → 

I agree that if there exited a "list of rights things to do", and it was
implemented, it may not be optimal to put it on M-RET [I'm not
sure]. . .

> Also, it is important to have a way to insert a headline whatever is
> around, and one to insert a headline at the end of the current section
> or even great-parent.

For the two latter: I only learned about the current possibility of
doing this reading the docstring of `org-insert-headline'.  I haven't
used it, and I don't think it's immediately helpful to me, but who
knows.

> Currently C-RET is sub-optimal since it is equivalent to C-u M-RET. It
> might be possible to re-define both M-RET and C-RET so they can cover
> all use-cases in a predictable, and meaningful, fashion.

Would be cool.

>> Here's another of my pet-griefs
>> - a
>> - b
>>
>> | → M-RET will give me an itme 
>> | → M-RET will give me a headline
>>
>> Why is the behavior a function of amount of whitespace/newlines to
>> nearest element?  This makes not sense to me and goes against what I
>> want, namely act in accordance to element at point. . .
>
> Blank lines belong to the element at point above.
>
> In particular, number of blank lines is meaningful in plain lists and
> footnote definitions (2 blank lines mark the end of the element). In
> the first line, you're still in the list, in the next one, you're not
> anymore, hence the behaviour.
>
> Think about
>
>   - a
>
>   - b

/I/ know why it does what it does.  But how about the guy who's been
using Org for five minutes?  Even knowing the technical/syntax reason, I
do not find this to be "predictable, and meaningful"—especially in my
initial example, less so when separating items by two lines.

Cheers,
Rasmus

-- 
Don't panic!!!

Re: M-RET vs C-RET

[Andreas Leha]

Hi,

[ ... ]


>
>>> Here's another of my pet-griefs
>>> - a
>>> - b
>>>
>>> | → M-RET will give me an itme 
>>> | → M-RET will give me a headline
>>>
>>> Why is the behavior a function of amount of whitespace/newlines to
>>> nearest element?  This makes not sense to me and goes against what I
>>> want, namely act in accordance to element at point. . .
>>
>> Blank lines belong to the element at point above.
>>
>> In particular, number of blank lines is meaningful in plain lists and
>> footnote definitions (2 blank lines mark the end of the element). In
>> the first line, you're still in the list, in the next one, you're not
>> anymore, hence the behaviour.
>>
>> Think about
>>
>>   - a
>>
>>   - b
>
> /I/ know why it does what it does.  But how about the guy who's been
> using Org for five minutes?  Even knowing the technical/syntax reason, I
> do not find this to be "predictable, and meaningful"—especially in my
> initial example, less so when separating items by two lines.

Just to add to that side thread:

I too fall regularly into that and have to undo, add more newlines and hit
M-RET again.  I have been using orgmode for quite some time and still
this is not 'predictable' for me.

Regards,
Andreas

Re: [patch] extend org-meta-return to keywords

[Rasmus]

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

Hi,

Thanks for the comments.

This email is long (sorry); fortunately there are many blank lines. . .

Thierry Banel <tbanelwebmin@free.fr> writes:
> If not, maybe it would be better to keep the oriGiNaL cASe.

Yeah, fixed.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>>> Moreover, it can get in the way of expected M-RET behaviour, as in the
>>> following example
>>>
>>>   - item
>>>
>>>   | #+caption: test
>>>     untenrsiu
>>
>> I don't know if I should do something about this case.  I guess it would
>> be possible, but I find it awkward when behavior $BUTTON depends not
>> only on not only the adjacent element, but also the previous element.
>>
>> If it's important, I guess it should be toggled explicitly on by a
>> custom variable.
>
> M-RET, is, first and foremost, an important keybinding for editing the
> /structure/ of the document. The behaviour you want to add has nothing
> to do with structure.

I see it differently.  I see M-RET as a function that "magically" adds
more of what is adjacent to point.

I—obviously—think what I propose is better than what we have now.  Let's
go through the current functionality.

* Example 1:

#+LATEX_HEADER: foo

Click M-RET anywhere above (line-beginning-position) and you get
something like

#+LATEX
* _HEADER: foo

This is nonsense.


Click M-RET at (line-beginning-position) and you get

* #+LATEX_HEADER: foo

This is nonsense.

* Example 2

- s

#+LATEX_HEADER: foo % no space between ^ and #.

Same as example 2.

* Next case

- s

 #+LATEX_HEADER: foo % space between ^ and # 

M-RET at a position greater than (line-beginning-position) yields
something like

- s

 #+LATEX

- _HEADER: tes

This is nonsense.


M-RET at (line-beginning-position) will yield

- s

- | ← that's point

 #+LATEX_HEADER: foo % space between ^ and #

Thus far this is the only sensible result that we'd loose with the
patch.  If this is a great loss (rather than a bug in the current
implementation) this can be dealt with, though it adds complexity to the
meaning of M-RET (since it's a function of the previous element rather
than the adjacent one).

* Example 4

- s


#+LATEX_HEADER: foo % three spaces

This works like example 1.

> As a consequence, I'm not sure it should go with M-RET, and if it does,
> I'm pretty sure it should not override the main purpose of the binding.
> Inserting headlines, and possibly items, is much more important than
> duplicating keywords.

IMO the examples above show that M-RET fails at doing this in a sensible
manner is all but one case above.  The case where it does not fail
requires (i) that the keyword is not at the beginning of the line and
(ii) that the use has point before the keyword.  I don't care strongly
about this "feature"—It's just check that point is not at bol in
`org-meta-return'.

In fact I would be happy to go further.  One also get nonsense result
doing M-RET on #+begin_src lines and probably elsewhere.

>> Also, `org-insert-keyword' is not really using org-element anymore.
>
> It should since you make it interactive and can, therefore, be called on
> its own.

I moved the check to a separate function,
`org-looking-at-repeatable-keyword-p'.  There are now the following
issues:

  1. it's called twice: once in org-meta-return and once in
     org-insert-keyword.  I'm not sure if it's possible to emit a signal
     to avoid the second check somehow...  Perhaps the performance-loss
     is such that we should not worry about it.

  2. The name is terrible.  I'm also not sure if this function should
     reside in org.el.  Same for the regexp.  Thoughts?

> Some comments follow:

Thanks for these!

>> +	 (indention
>> +	  (buffer-substring
>> +	   (line-beginning-position)
>> +	   (save-excursion
>> +	     (beginning-of-line)
>> +	     (skip-chars-forward " \t")
>> +	     (point))))
>
> Another option:
>
>   (ind (org-get-indentation))

Cool.  I really need to internalize to look for org-prefixed functions!


>> +	 (keyword-re "#\\+\\(.+?\\):")
>> +	 (key (save-excursion (beginning-of-line)
>> +			      (skip-chars-forward " \t")
>> +			      (looking-at keyword-re)
>> +			      (upcase (org-match-string-no-properties 1))))
>
> As discussed above, this is too fragile.  You need to duplicate the
> checks done in `org-meta-return' or refactor the code.

Yeah, I see your point.  Casual testing suggest it's more solid now
albeit there's now the double check as outlined above.
 
>> +	 (end-of-keyword (save-excursion
>> +			   (beginning-of-line)
>> +			   (re-search-forward keyword-re (line-end-position) t)
>> +			   (point))))
>
>   (end-of-keyword (save-excursion (beginning-of-line) (search-forward ":"))
> [...]
> I think end-of-keyword should be bound after KEY is checked to avoid
> errors.

Since there's now a check before entering the function I guess this
should be safe.  Still I added extra checks, that I'm pretty sure are
unnecessary.  I'd be happy to remove these.

—Rasmus

-- 
Don't panic!!!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Add-keyword-support-to-M-RET.patch --]
[-- Type: text/x-diff, Size: 6838 bytes --]

From 7443986c8d10efc7ee3216c3dbac6a5ace8a4468 Mon Sep 17 00:00:00 2001
From: rasmus <rasmus@gmx.us>
Date: Wed, 19 Nov 2014 15:39:19 +0100
Subject: [PATCH] org.el: Add keyword-support to M-RET

* org.el (org-keyword-regexp): Regexp to detect keyword.
(org-looking-at-repeatable-keyword-p): New function.
(org-insert-keyword): New function.
(org-meta-return): May call `org-insert-keyword'.
(org-M-RET-may-split-line): Add keyword.
---
 lisp/org.el | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 101 insertions(+), 11 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 6ab13f4..6e75db4 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -440,6 +440,12 @@ Matched keyword is in group 1.")
   (concat "\\<" org-closed-string " *\\[\\([^]]+\\)\\]")
   "Matches the CLOSED keyword together with a time stamp.")
 
+(defconst org-keyword-regexp
+  "^[ \t]*#\\+\\(.+?\\):"
+  "Matches keywords such as #+LATEX_HEADER: and #+CAPTION:.
+
+See also `org-looking-at-repeatable-keyword-p'.")
+
 (defconst org-keyword-time-regexp
   (concat "\\<"
 	  (regexp-opt
@@ -1601,6 +1607,7 @@ contexts.  Valid contexts are:
 headline  when creating a new headline
 item      when creating a new item
 table     in a table field
+keyword   when creating a new keyword
 default   the value to be used for all contexts not explicitly
           customized"
   :group 'org-structure
@@ -1614,6 +1621,7 @@ default   the value to be used for all contexts not explicitly
 			   (const headline)
 			   (const item)
 			   (const table)
+			   (const keyword)
 			   (const default))
 		   (boolean)))))
 
@@ -7725,8 +7733,8 @@ split the line and create a new headline with the text in the
 current line after point \(see `org-M-RET-may-split-line' on how
 to modify this behavior).
 
-If point is at the beginning of a normal line, turn this line
-into a heading.
+If point is at the beginning of a normal line, excluding some
+keywords, turn this line into a heading.
 
 When INVISIBLE-OK is set, stop at invisible headlines when going
 back.  This is important for non-interactive uses of the
@@ -21291,10 +21299,90 @@ number of stars to add."
 	       (forward-line)))))))
     (unless toggled (message "Cannot toggle heading from here"))))
 
+(defun org-looking-at-repeatable-keyword-p ()
+  "Return non-nil if point is at repeatable keyword.
+
+Repeatable keyword are either keywords that are not members of
+`org-element-affiliated-keywords' or affiliated keywords that are
+members of `org-element-multiple-keywords'."
+  (let ((element (org-element-at-point))
+	(type (org-element-type element)))
+    (or (eq type 'keyword)
+	(<  (point) (org-element-property :post-affiliated element)))
+    (let ((key (save-excursion
+		 (beginning-of-line)
+		 (and (looking-at org-keyword-regexp)
+		      (org-match-string-no-properties 1)))))
+      (and key
+	   (or (not (member-ignore-case
+		     key org-element-affiliated-keywords))
+	       (member-ignore-case
+		key org-element-multiple-keywords))))))
+
+(defun org-insert-keyword (&optional arg)
+  "Insert a new keyword-line, such as #+CAPTION or #+LATEX_HEADER.
+
+If point is between the beginning of the line and the beginning
+of the keyword, as denoted by \"#\", a keyword-line is inserted
+above the current one.  If the point is within the keyword, a new
+keyword-line is inserted below.
+
+If point is in the middle of a keyword-line, after the keyword
+itself, split the line depending on the value of
+`org-M-RET-may-split-line'.  See the docstring of this variable
+for further details.
+
+Currently arg is ignored and the keyword is determined from the
+context.
+
+The function is used by `org-meta-return'.  Note, affiliated
+keywords that are not allowed to appear multiple times are
+ignored by `org-meta-return' (see
+`org-element-affiliated-keywords' and
+`org-element-multiple-keywords')."
+  (interactive "P")
+  (when (org-looking-at-repeatable-keyword-p)
+    (let* ((may-split (org-get-alist-option
+		       org-M-RET-may-split-line 'keyword))
+	   (point-at-bol-p
+	    (save-excursion (skip-chars-backward " \t") (bolp)))
+	   (indention (org-get-indentation))
+	   (key (save-excursion (beginning-of-line)
+				(and (looking-at org-keyword-regexp)
+				     (org-string-nw-p (org-match-string-no-properties 1)))))
+	   (end-of-keyword (and key
+				(save-excursion
+				  (beginning-of-line)
+				  (search-forward ":" (line-end-position) t)
+				  (point)))))
+      (when key
+	(cond (point-at-bol-p
+	       ;; Point is before keyword.
+
+	       ;; TODO: Perhaps it should call `org-insert-heading'
+	       ;; when point-at-bol-p.  If so this check should be in
+	       ;; `org-meta-return'.
+
+	       ;; TODO: it would be nice to have the insert \n after
+	       ;; the cond, but the save-excursion would somehow need
+	       ;; to be applied...
+	       (save-excursion
+		 (beginning-of-line)
+		 (insert "\n")))
+	      ((or (not may-split)
+		   (< (point) end-of-keyword))
+	       (end-of-line)
+	       (insert "\n"))
+	      (t (insert "\n")))
+	(org-indent-line-to indention)
+	(insert (format "#+%s: " key))
+	(delete-region (point) (save-excursion (skip-chars-forward " \t") (point)))))))
+
 (defun org-meta-return (&optional arg)
-  "Insert a new heading or wrap a region in a table.
-Calls `org-insert-heading' or `org-table-wrap-region', depending
-on context.  See the individual commands for more information."
+  "Insert a new heading, a new keyword or wrap a region in a table.
+Calls `org-insert-heading', `org-insert-keyword' or
+`org-table-wrap-region', depending on context.  See the
+individual commands for more information."
   (interactive "P")
   (org-check-before-invisible-edit 'insert)
   (or (run-hook-with-args-until-success 'org-metareturn-hook)
@@ -21303,12 +21391,14 @@ on context.  See the individual commands for more information."
         (when (eq type 'table-row)
           (setq element (org-element-property :parent element))
           (setq type 'table))
-        (if (and (eq type 'table)
-                 (eq (org-element-property :type element) 'org)
-                 (>= (point) (org-element-property :contents-begin element))
-                 (< (point) (org-element-property :contents-end element)))
-            (call-interactively 'org-table-wrap-region)
-          (call-interactively 'org-insert-heading)))))
+        (cond  ((and (eq type 'table)
+		      (eq (org-element-property :type element) 'org)
+		      (>= (point) (org-element-property :contents-begin element))
+		      (< (point) (org-element-property :contents-end element)))
+		(call-interactively 'org-table-wrap-region))
+	       ((org-looking-at-repeatable-keyword-p)
+		(call-interactively 'org-insert-keyword))
+	       (t (call-interactively 'org-insert-heading))))))
 
 ;;; Menu entries
 
-- 
2.1.3

Re: [patch] extend org-meta-return to keywords

[Nicolas Goaziou]

Rasmus <rasmus@gmx.us> writes:

> I see it differently.  I see M-RET as a function that "magically" adds
> more of what is adjacent to point.

Your "definition" is fuzzy:

 * Headline

   [lots of text]

   <-- M-RET here will insert a headline, which is by no mean adjacent to point.


> I—obviously—think what I propose is better than what we have now.  Let's
> go through the current functionality.

In this case I value simplicity over complexity (or "smartness",
whatever you call it). Anyone, including newcomers, should be able to
use M-RET without knowing about (affiliated) keywords, or be bothered
with them.

IMO, it /is/ sensible to insert a headline (or an item) without trying
to outsmart the user. 

Also, headlines are first-class citizens in Org. Keybindings acting on
them shouldn't be bothered with other citizens.


Regards,

Re: [Prelim. patch] extend org-meta-return to keywords

[Thierry Banel]

M-RET working "as expected" is appealing.
Your patch makes M-RET moreuseful than it is now.
I vote +1.

There are a few issues thought.
I guess they can be worked out.

For instance:
   #+HEA|DER: :var v="hello"     (cursor is the pipe |)
ischanged into
   #+HEA
   #+HEADER: |DER: :var v="hello"
which is pretty useless

Also if #+HEADER: is immediately followed by a #+BEGIN_SRC,
the patch (for an unknown reason) is ignored,
and the old behavior applies.

Thanks for this
Thierry


Le 19/11/2014 15:41, Rasmus a écrit :
> Hi,
>
> Something I have wanted for a while is to have M-RET work "as
> expected"(?)  on keyword lines such as #+LATEX_HEADER, #+CAPTION etc.
>
> An "animation" to illustrate, where '|' is point:
>
>     #+CAPTION: this is a |long caption
>
>     #   click <M-RET> ⇒ 
>
>     #+CAPTION: this is a 
>     #+CAPTION: |long caption
>
> Would anyone else like this?
>
> Attached is a quick patch that works surprisingly well.  I would work
> more on it if you guys agree this would be useful.  It probably need
> much more work for corner-cases (any ideas what these are)?
>
> Thanks,
> Rasmus
>

Re: [Prelim. patch] extend org-meta-return to keywords

[Rasmus]

Hi Thierry,

Thanks for the feedback.
I posted an updated patch under the subject:

  [patch] extend org-meta-return to keywords

where Nicolas commented.

Thierry Banel <tbanelwebmin@free.fr> writes:

> M-RET working "as expected" is appealing.
> Your patch makes M-RET moreuseful than it is now.
> I vote +1.

> There are a few issues thought.
> I guess they can be worked out.
>
> For instance:
>    #+HEA|DER: :var v="hello"     (cursor is the pipe |)
> ischanged into
>    #+HEA
>    #+HEADER: |DER: :var v="hello"
> which is pretty useless

Thanks.  I'm pretty sure it's fixed in the version from last night.

> Also if #+HEADER: is immediately followed by a #+BEGIN_SRC,
> the patch (for an unknown reason) is ignored,
> and the old behavior applies.

OK, that's a good test case.  I will look into it.

Thanks,
Rasmus

-- 
Enough with the bla bla!

Re: [Prelim. patch] extend org-meta-return to keywords

[Thierry Banel]

Le 22/11/2014 15:31, Rasmus a écrit :

>> There are a few issues thought.
>> I guess they can be worked out.
>>
>> For instance:
>>    #+HEA|DER: :var v="hello"     (cursor is the pipe |)
>> ischanged into
>>    #+HEA
>>    #+HEADER: |DER: :var v="hello"
>> which is pretty useless
> Thanks.  I'm pretty sure it's fixed in the version from last night.
Sorry, Rasmus, I should have noticed your second patch.
First issue fixed!
Regard, Thierry