[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [patch, ox] #+INCLUDE resolves links
From: |
Nicolas Goaziou |
Subject: |
Re: [O] [patch, ox] #+INCLUDE resolves links |
Date: |
Wed, 24 Sep 2014 23:22:50 +0200 |
Hello,
Rasmus <address@hidden> writes:
> Okay, I hope I got a better patch here.
Thank you. Some comments follow.
> Nicolas Goaziou <address@hidden> writes:
>> This should be ":only-contents t" or ":only-contents nil".
>> ":only-contents" alone can be tolerated as a shortcut for
>> ":only-contents nil", but that's all.
>
> 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.
> It doesn't make sense to apply this to source files, right?
Right.
> +elements.}. If the keyword @code{:only-contents} is used, only the contents
> +of the included element.
The sentence is not complete. Also, it should be something like "If you
set @code{:only-contents} property to a non-nil value, only...".
> For headlines, drawers and properties
> +immediately following the headline will not be included when using
> address@hidden:only-contents}.
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).
> Some examples:
> +
> address@hidden
> +#+INCLUDE: "./paper.org::#theory" :only-contents
#+INCLUDE: "./paper.org::#theory" :only-contents t
> address@hidden
> +#+INCLUDE: "./otherfile.org::#my_custom_id" :no-contents
> address@hidden smallexample
#+INCLUDE: "./otherfile.org::#my_custom_id" :only-contents t
> + (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)))
> + (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))))
> + (let ((org-inhibit-startup t)
> + (lines (org-export--inclusion-absolute-lines
> + file location only-contents lines)))
Lines are local to the element only if LOCATION is provided, right?
(lines (if location
(org-export--inclusion-absolute-lines ...)
lines))
> + (with-temp-buffer
> + (insert-file-contents file)
> + (when location
This check is useless since we know location is defined (otherwise,
this function wouldn't be called).
> + ;; locations are only defined for org files so
> + ;; OK to start org-mode.
You can remove this.
> + (condition-case err
> + ;; enforce consistency in search.
"Enforce"
> + (let ((org-link-search-must-match-exact-headline t))
> + (org-link-search location))
> + ;; helpful error messages
You can remove this.
> + (error
> + (error (format "%s for %s::%s" (error-message-string err) file
> location))))
> + (org-mode)
Shouldn't (org-mode) be moved before calling `org-link-search'? At some
point, `org-link-search' may use Elements.
> + (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))))
> + (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'.
> + (narrow-to-region beg end)))
This is not needed.
> + (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.
> + (switch-to-buffer-other-window (current-buffer))
> + (insert-file-contents file)
Why is it needed?
> (when ind
> (unless (eq major-mode 'org-mode)
> (let ((org-inhibit-startup t)) (org-mode)))
> + ;; make sure that there is not an immediate "lonely"
> + ;; property-drawer. Normal drawers are OK but property-drawers
> + ;; may follow normal drawers.
> + (goto-char (point-min))
> + (let ((element (org-element-at-point)))
> + (while (memq (org-element-type element) '(drawer property-drawer))
> + ;; entering here the first element is not a heading so it's
> + ;; safe to get rid of property-drawers.
> + (if (eq (org-element-type element) 'property-drawer)
> + (delete-region (org-element-property :begin element)
> + (org-element-property :end element))
> + (goto-char (org-element-property :end element)))
> + (setq element (org-element-at-point))))
Why do you need to skip anything since function above already took care
of that?
Regards,
--
Nicolas Goaziou
- [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 <=
- Re: [O] [patch, ox] #+INCLUDE resolves links, Rasmus, 2014/09/28
- 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