Hi, Thanks for the comments. This email is long (sorry); fortunately there are many blank lines. . . Thierry Banel writes: > If not, maybe it would be better to keep the oriGiNaL cASe. Yeah, fixed. Nicolas Goaziou writes: > Rasmus 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!!!