From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [patch] extend org-meta-return to keywords Date: Sun, 23 Nov 2014 00:20:25 +0100 Message-ID: <87bnny4ybq.fsf@nicolasgoaziou.fr> References: <87egszw8ui.fsf@gmx.us> <87wq6o3u57.fsf@gmx.us> <87fvdb4l6c.fsf@nicolasgoaziou.fr> <87sihb4ede.fsf@pank.eu> <87lhn36nq1.fsf@gmx.us> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsJxz-0005yQ-P9 for emacs-orgmode@gnu.org; Sat, 22 Nov 2014 18:19:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XsJxp-0000PI-SO for emacs-orgmode@gnu.org; Sat, 22 Nov 2014 18:19:47 -0500 Received: from relay3-d.mail.gandi.net ([2001:4b98:c:538::195]:52801) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XsJxp-0000No-JK for emacs-orgmode@gnu.org; Sat, 22 Nov 2014 18:19:37 -0500 In-Reply-To: <87lhn36nq1.fsf@gmx.us> (rasmus@gmx.us's message of "Sat, 22 Nov 2014 20:26:30 +0100") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Rasmus Cc: tbanelwebmin@free.fr, emacs-orgmode@gnu.org 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. 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,