[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [patch, ox] #+INCLUDE resolves links
From: |
Rasmus |
Subject: |
Re: [O] [patch, ox] #+INCLUDE resolves links |
Date: |
Sun, 28 Sep 2014 21:32:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) |
Hi,
Thanks for the comments. I hope I addressed the previous comments and
did not introduce new reasons bugs.
I added tests.
Comments on comments follow.
Nicolas Goaziou <address@hidden> writes:
>> Okay, I hope I got it now. It's a rather forgiving regexp in terms of
>> mistakes. Is that OK?
>
> Please no ":only-contents yes", ":only-contents true", ":only-contents
> of_course!" in the regexp. If :only-contents is followed by anything but
> nil or another keyword, its value is non-nil. See below.
Good catch; I added explicit support for
¡?[oO][fF][-_]?[cC][Oo][uU][rR][sS][\.!¡]?
in the regexp! ¡Gotta catch 'em all!
> The sentence is not complete. Also, it should be something like "If you
> set @code{:only-contents} property to a non-nil value, only...".
>> [...]
> This is not true anymore about the drawers. This should be merged with
> the previous phrase to avoid duplicating "@code{:only-contents}" (e.g.,
> only the contents of the matched element are inserted, without any
> planning line or property drawer).
Fixed this and other documentation bugs — hopefully. Let me know if
it's clear.
>> + (let ((matched (save-match-data
>> + (org-split-string
>> + (org-remove-double-quotes (match-string 1 value)) "::"))))
>
> There's no reason to use `org-split-string' here since you only want to
> match the last "::". You can use the same regexp used by
> "org-element.el", i.e.
>
> (when (string-match "::\\(.*\\)\\'" value)
> (setq location (match-string 1 value)
> value (replace-match "" nil nil value)))
Custom_ID is very flexible. I've use a similar regexp.
>> + (only-contents
>> + (and (string-match
>> + ":only-?contents?[[:space:]]*\"?\\(t\\|true\\|yes\\)?\"?"
>> + value)
>> + (prog1 (and (match-string 1 value) t)
>> + (setq value (replace-match "" nil nil value)))))
>
> (only-contents
> (and (string-match ":only-contents +\\([^: \r\t\n]\\S-*\\)" value)
> (org-not-nil (match-string 1 value))))
I have removed flexibility in speling.
>> + (narrow-to-region
>> + (org-element-property
>> + (if only-contents :contents-begin :begin) (org-element-at-point))
>> + (org-element-property (if only-contents :contents-end :end)
>> (org-element-at-point))))
>
> (let ((element (org-element-at-point)))
> (let ((contents-beg
> (and only-contents
> (org-element-property :contents-begin element))))
> (narrow-to-region
> (or contents-beg (org-element-property :begin element))
> (org-element-property (if contents-beg :contents-end :end) element))))
Just out of curiosity, what is an example of a element that can be
named and does not have a :contents-begin?
>> + (when only-contents
>> + ;; skip drawers and property-drawers
>> + ;; these are removed as needed in `org-export--prepare-file-contents'
>> + ;; TODO: How to actually do this? Only line numbers are send to
>> + ;; `org-export--prepare-file-contents'. split in two?
>> + (goto-char (point-min))
>> + (org-skip-whitespace)
>> + (beginning-of-line)
>> + (let ((element (org-element-at-point)))
>> + (while (memq (org-element-type element) '(drawer property-drawer))
>> + (goto-char (org-element-property :end element))
>> + (setq element (org-element-at-point)))))
>
> Regular drawers are not expected to be skipped. Also, the following
> should be better
>
> (when (and only-contents
> (memq (org-element-type element) '(headline inlinetask)))
> (goto-char (point-min))
> (when (org-looking-at-p org-planning-line-re) (forward-line))
> (when (looking-at org-property-drawer-re) (goto-char (match-end 0)))
> (unless (bolp) (forward-line)))
>
> This should be obviously included within the previous `let'.
Okay, there's a lot of improvements in that suggestion. However, it
misses this case which created using only "official" shortcuts
* head
SCHEDULED: <2014-09-28 sun>
:LOGBOOK:
- Note taken on [2014-09-28 sat 12:21] \\
a drawer
:END:
:PROPERTIES:
:CUSTOM_ID: h
:END:
The patch handles something like this now cf. the last test.
>> + (apply (lambda (beg end) (format "%s-%s" beg end))
>> + ;; `line-number-at-pos' returns the narrowed line-number
>> + (mapcar 'line-number-at-pos (prog1 (list (point-min) (point-max))
>> (widen))))))
>
> This is inefficient because `line-number-at-pos' will start counting
> twice from line 1.
>
> (goto-char beg)
> (widen)
> (let ((start-line (line-number-at-pos)))
> (format "%d-%d"
> start-line
> (+ start-line
> (let ((c 0)) (while (< (point) end) (incf c) (forward-line))
> c))))
>
> I didn't check, there may an off-by-one error. Anyway, all this needs
> tests.
Fine with me. It's a bit less elegant IMO, but you are right. I had
to do it slightly differently since the line number needs to appear
irrespective of whether lines are included in the call initially.
That being said, I could have very well overlooked some obvioues way
of doing it.
—Rasmus
--
Hooray!
0001-ox-Allow-file-links-with-INCLUDE-keyword.patch
Description: Text Data
- [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/20
- Re: [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/21
- Re: [O] [patch, ox] #+INCLUDE resolves links, Nicolas Goaziou, 2014/09/21
- Re: [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/23
- Re: [O] [patch, ox] #+INCLUDE resolves links, Nicolas Goaziou, 2014/09/24
- Re: [O] [patch, ox] #+INCLUDE resolves links,
Rasmus <=
- Re: [O] [patch, ox] #+INCLUDE resolves links, Nicolas Goaziou, 2014/09/30
- Re: [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/30
- Re: [O] [patch, ox] #+INCLUDE resolves links, Nicolas Goaziou, 2014/09/30
- Re: [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/30