From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Rasmus <rasmus@gmx.us>
Cc: emacs-orgmode@gnu.org
Subject: Re: [patch, ox] #+INCLUDE resolves links
Date: Wed, 24 Sep 2014 23:22:50 +0200 [thread overview]
Message-ID: <87oau4ems5.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87bnq5zzp7.fsf@gmx.us> (rasmus@gmx.us's message of "Wed, 24 Sep 2014 01:25:56 +0200")
Hello,
Rasmus <rasmus@gmx.us> writes:
> Okay, I hope I got a better patch here.
Thank you. Some comments follow.
> Nicolas Goaziou <mail@nicolasgoaziou.fr> 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
> +@code{: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:
> +
> +@example
> +#+INCLUDE: "./paper.org::#theory" :only-contents
#+INCLUDE: "./paper.org::#theory" :only-contents t
> +@smallexample
> +#+INCLUDE: "./otherfile.org::#my_custom_id" :no-contents
> +@end 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
next prev parent reply other threads:[~2014-09-24 21:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-21 0:51 [patch, ox] #+INCLUDE resolves links Rasmus
2014-09-21 11:46 ` Rasmus
2014-09-21 13:53 ` Nicolas Goaziou
2014-09-21 14:46 ` Rasmus
2014-09-21 19:51 ` Nicolas Goaziou
2014-09-23 23:25 ` Rasmus
2014-09-24 21:22 ` Nicolas Goaziou [this message]
2014-09-28 19:32 ` Rasmus
2014-09-30 8:07 ` Nicolas Goaziou
2014-09-30 10:18 ` Rasmus
2014-09-30 14:29 ` Nicolas Goaziou
2014-09-30 21:48 ` Rasmus
2014-10-01 20:03 ` Nicolas Goaziou
2014-10-01 21:27 ` Rasmus
2014-10-02 7:29 ` Xavier Garrido
2014-10-02 8:55 ` Rasmus
2014-10-02 16:30 ` Aaron Ecay
2014-10-02 16:53 ` Nicolas Goaziou
2014-10-02 17:47 ` Rasmus
2014-10-02 19:11 ` Achim Gratz
2014-10-02 20:58 ` Rasmus
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=87oau4ems5.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=rasmus@gmx.us \
/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).