emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Rasmus <rasmus@gmx.us>
To: emacs-orgmode@gnu.org
Subject: Re: [patch] extend org-meta-return to keywords
Date: Sun, 23 Nov 2014 18:00:36 +0100	[thread overview]
Message-ID: <87bnnx7sy3.fsf@gmx.us> (raw)
In-Reply-To: 87bnny4ybq.fsf@nicolasgoaziou.fr

[-- 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


  parent reply	other threads:[~2014-11-23 17:01 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19 14:41 [Prelim. patch] extend org-meta-return to keywords Rasmus
2014-11-22  1:23 ` [patch] " Rasmus
2014-11-22  9:52   ` Nicolas Goaziou
2014-11-22 12:19     ` Rasmus Pank Roulund
2014-11-22 19:26       ` Rasmus
2014-11-22 21:57         ` Thierry Banel
2014-11-22 23:20         ` Nicolas Goaziou
2014-11-23 11:08           ` Thierry Banel
2014-11-23 16:54             ` Nicolas Goaziou
2014-11-23 17:15               ` Rasmus
2014-11-23 17:54                 ` Nicolas Goaziou
2014-11-23 18:11                   ` Rasmus
2014-11-23 21:13                     ` Nicolas Goaziou
2014-11-23 21:54                       ` Rasmus
2014-11-25  9:00                         ` Nicolas Goaziou
2014-11-25 10:12                           ` M-RET vs C-RET Sebastien Vauban
2014-11-25 11:09                             ` Nicolas Goaziou
2014-11-25 11:16                             ` Rasmus
2014-11-26 23:50                               ` Nicolas Goaziou
2014-11-27  0:55                                 ` Rasmus
2014-11-27  9:19                                   ` Andreas Leha
2014-11-23 17:00           ` Rasmus [this message]
2014-11-23 17:46             ` [patch] extend org-meta-return to keywords Nicolas Goaziou
2014-11-22 13:53 ` [Prelim. patch] " Thierry Banel
2014-11-22 14:31   ` Rasmus
2014-11-22 17:09     ` Thierry Banel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bnnx7sy3.fsf@gmx.us \
    --to=rasmus@gmx.us \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).