From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [patch, ox] #+INCLUDE resolves links Date: Wed, 24 Sep 2014 23:22:50 +0200 Message-ID: <87oau4ems5.fsf@nicolasgoaziou.fr> References: <87k34x6bjd.fsf@gmx.us> <87lhpdurfh.fsf@gmx.us> <87bnq984hd.fsf@nicolasgoaziou.fr> <87bnq5zzp7.fsf@gmx.us> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:33179) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWu0z-0005Ee-9D for emacs-orgmode@gnu.org; Wed, 24 Sep 2014 17:22:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWu0s-0006fX-H1 for emacs-orgmode@gnu.org; Wed, 24 Sep 2014 17:22:21 -0400 Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:53797) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWu0s-0006em-89 for emacs-orgmode@gnu.org; Wed, 24 Sep 2014 17:22:14 -0400 In-Reply-To: <87bnq5zzp7.fsf@gmx.us> (rasmus@gmx.us's message of "Wed, 24 Sep 2014 01:25:56 +0200") 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: emacs-orgmode@gnu.org Hello, Rasmus writes: > Okay, I hope I got a better patch here. Thank you. Some comments follow. > Nicolas Goaziou 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