* [RFC] [PATCH] Warn about unexpanded macros on export @ 2014-09-19 19:12 Aaron Ecay 2014-09-19 19:29 ` Nicolas Goaziou 0 siblings, 1 reply; 21+ messages in thread From: Aaron Ecay @ 2014-09-19 19:12 UTC (permalink / raw) To: Org-mode [-- Attachment #1: Type: text/plain, Size: 1237 bytes --] Hello all, Currently, if a macro is not defined, it will silently produce an empty string while exporting. This situation could arise for example if a macro name is acidentally mistyped. I think it’s desirable to warn the user in this case. The attached patch introduces a function to do so, and shows how it would be integrated in the latex backend. This raises several questions: 1. Should the warning be a “message” (allows the export process to continue) or a “user-error” (stops the export process)? Or, should this be configurable? 2. Since this is a feature that many backends will want to use, should we introduce a new “abstract” backend from which other backends can inherit, which incorporates this feature, and others like it in the future? The idea would be similar to prog-mode in emacs, the major mode from which programming-language modes can derive. The alternative is adding the (macro . org-export-macro-warn) entry manually to all the relevant backends, and relying on future backend authors to do the same. 3. Should this even be implemented as part of the backend’s translate-alist, or at a lower level? Thanks, -- Aaron Ecay [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ox-warn-if-unexpanded-macros-are-found-when-exportin.patch --] [-- Type: text/x-diff, Size: 1324 bytes --] From 1c9f85bcb93dbc56d01b138f5a4a11ad0933b5c6 Mon Sep 17 00:00:00 2001 From: Aaron Ecay <aaronecay@gmail.com> Date: Fri, 19 Sep 2014 14:46:37 -0400 Subject: [PATCH] ox: warn if unexpanded macros are found when exporting * lisp/ox.el (org-export-macro-warn): New function. * lisp/ox-latex.el: Use it. --- lisp/ox-latex.el | 1 + lisp/ox.el | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/lisp/ox-latex.el b/lisp/ox-latex.el index f59d6b2..7670ccb 100644 --- a/lisp/ox-latex.el +++ b/lisp/ox-latex.el @@ -65,6 +65,7 @@ (latex-fragment . org-latex-latex-fragment) (line-break . org-latex-line-break) (link . org-latex-link) + (macro . org-export-macro-warn) (node-property . org-latex-node-property) (paragraph . org-latex-paragraph) (plain-list . org-latex-plain-list) diff --git a/lisp/ox.el b/lisp/ox.el index f01f951..a4988f4 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -5619,6 +5619,11 @@ to `:default' encoding. If it fails, return S." (plist-get translations :default) s))) +(defun org-export-macro-warn (macro contents info) + ;; TODO: should this be a user-error? + (message "WARNING: undefined macro %s" (org-element-property :key macro)) + ;; Return empty string to avoid interfering with the export output. + "") \f ;;; Asynchronous Export -- 2.1.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-19 19:12 [RFC] [PATCH] Warn about unexpanded macros on export Aaron Ecay @ 2014-09-19 19:29 ` Nicolas Goaziou 2014-09-23 3:25 ` Aaron Ecay 0 siblings, 1 reply; 21+ messages in thread From: Nicolas Goaziou @ 2014-09-19 19:29 UTC (permalink / raw) To: Org-mode Hello, Aaron Ecay <aaronecay@gmail.com> writes: > Currently, if a macro is not defined, it will silently produce an empty > string while exporting. This situation could arise for example if a > macro name is acidentally mistyped. I think it’s desirable to warn the > user in this case. The attached patch introduces a function to do so, > and shows how it would be integrated in the latex backend. This raises > several questions: > > 1. Should the warning be a “message” (allows the export process to > continue) or a “user-error” (stops the export process)? Or, should > this be configurable? Certainly not a message, due to asynchronous export. > 2. Since this is a feature that many backends will want to use, should > we introduce a new “abstract” backend from which other backends can > inherit, which incorporates this feature, and others like it in the > future? The idea would be similar to prog-mode in emacs, the major > mode from which programming-language modes can derive. The > alternative is adding the (macro . org-export-macro-warn) entry > manually to all the relevant backends, and relying on future backend > authors to do the same. > > 3. Should this even be implemented as part of the backend’s > translate-alist, or at a lower level? Don't bother with export back-ends, they never get to see macros, which are expanded very early in the export process. This explains, for example, why there is no `macro' translator. You have two options. Either report an error, as you suggested, or insert an obnoxious message in the output, e.g., "UNKNOWN MACRO", à la "DEFINITION NOT FOUND." for footnote definitions. In any case, this should happen in "org-macro.el", not in the export framework. Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-19 19:29 ` Nicolas Goaziou @ 2014-09-23 3:25 ` Aaron Ecay 2014-09-23 16:59 ` Grant Rettke ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Aaron Ecay @ 2014-09-23 3:25 UTC (permalink / raw) To: Nicolas Goaziou, Org-mode Hi Nicolas, 2014ko irailak 19an, Nicolas Goaziou-ek idatzi zuen: > > Certainly not a message, due to asynchronous export. Very good point. > >> 2. Since this is a feature that many backends will want to use, should >> we introduce a new “abstract” backend from which other backends can >> inherit, which incorporates this feature, and others like it in the >> future? The idea would be similar to prog-mode in emacs, the major >> mode from which programming-language modes can derive. The >> alternative is adding the (macro . org-export-macro-warn) entry >> manually to all the relevant backends, and relying on future backend >> authors to do the same. >> >> 3. Should this even be implemented as part of the backend’s >> translate-alist, or at a lower level? > > Don't bother with export back-ends, they never get to see macros, which > are expanded very early in the export process. This explains, for > example, why there is no `macro' translator. Um...but the patch I sent works precisely by defining a macro translator, which does get called for any unexpanded (because undefined) macros. I guess you’re saying the code ought to block this at a lower/earlier level. > > You have two options. Either report an error, as you suggested, or > insert an obnoxious message in the output, e.g., "UNKNOWN MACRO", à la > "DEFINITION NOT FOUND." for footnote definitions. In any case, this > should happen in "org-macro.el", not in the export framework. I think error is better than obnoxious message, because it’s possible for the latter to slip through into a “production” document. (We ought to proofread our documents carefully, of course...but no one’s perfect). One issue is that the exporter does two macro expansion passes – one for garden-variety macros, and the second for author, date, email, and title. So, we can’t make the macro expansion unconditionally barf on undefined macros (since for the first pass e.g. author is undefined). I see three options: 1. explicitly whitelist the few “blessed” macros like author, and error on any other undefined macro 2. add an optional “final” arg to org-macro-replace-all, which will activate the undefined checking only if non-nil, and pass this flag in the exporter’s second (and last) call to org-macro-replace-all 3. in ‘org-export-as’, manually walk the parse tree after expanding macros, and make sure no 'macro type objects are left WDYT? -- Aaron Ecay ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 3:25 ` Aaron Ecay @ 2014-09-23 16:59 ` Grant Rettke 2014-09-23 17:18 ` Aaron Ecay 2014-09-23 20:24 ` Nicolas Goaziou 2014-09-23 20:26 ` Rasmus 2 siblings, 1 reply; 21+ messages in thread From: Grant Rettke @ 2014-09-23 16:59 UTC (permalink / raw) To: Nicolas Goaziou, Org-mode Aaron may I trouble you to add a flag so that if such errors occur then the export fails? From my perspective, if the document doesn't "compile", then nothing should succeed. Compile includes export from my perspective. On Mon, Sep 22, 2014 at 10:25 PM, Aaron Ecay <aaronecay@gmail.com> wrote: > Hi Nicolas, > > 2014ko irailak 19an, Nicolas Goaziou-ek idatzi zuen: >> >> Certainly not a message, due to asynchronous export. > > Very good point. > >> >>> 2. Since this is a feature that many backends will want to use, should >>> we introduce a new “abstract” backend from which other backends can >>> inherit, which incorporates this feature, and others like it in the >>> future? The idea would be similar to prog-mode in emacs, the major >>> mode from which programming-language modes can derive. The >>> alternative is adding the (macro . org-export-macro-warn) entry >>> manually to all the relevant backends, and relying on future backend >>> authors to do the same. >>> >>> 3. Should this even be implemented as part of the backend’s >>> translate-alist, or at a lower level? >> >> Don't bother with export back-ends, they never get to see macros, which >> are expanded very early in the export process. This explains, for >> example, why there is no `macro' translator. > > Um...but the patch I sent works precisely by defining a macro translator, > which does get called for any unexpanded (because undefined) macros. I > guess you’re saying the code ought to block this at a lower/earlier level. > >> >> You have two options. Either report an error, as you suggested, or >> insert an obnoxious message in the output, e.g., "UNKNOWN MACRO", à la >> "DEFINITION NOT FOUND." for footnote definitions. In any case, this >> should happen in "org-macro.el", not in the export framework. > > I think error is better than obnoxious message, because it’s possible > for the latter to slip through into a “production” document. (We ought > to proofread our documents carefully, of course...but no one’s perfect). > > One issue is that the exporter does two macro expansion passes – one > for garden-variety macros, and the second for author, date, email, and > title. So, we can’t make the macro expansion unconditionally barf on > undefined macros (since for the first pass e.g. author is undefined). > I see three options: > 1. explicitly whitelist the few “blessed” macros like author, and error > on any other undefined macro > 2. add an optional “final” arg to org-macro-replace-all, which will > activate the undefined checking only if non-nil, and pass this flag > in the exporter’s second (and last) call to org-macro-replace-all > 3. in ‘org-export-as’, manually walk the parse tree after expanding > macros, and make sure no 'macro type objects are left > > WDYT? > > -- > Aaron Ecay > -- Grant Rettke gcr@wisdomandwonder.com | http://www.wisdomandwonder.com/ “Wisdom begins in wonder.” --Socrates ((λ (x) (x x)) (λ (x) (x x))) “Life has become immeasurably better since I have been forced to stop taking it seriously.” --Thompson ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 16:59 ` Grant Rettke @ 2014-09-23 17:18 ` Aaron Ecay 2014-09-23 17:29 ` Jacob Gerlach 0 siblings, 1 reply; 21+ messages in thread From: Aaron Ecay @ 2014-09-23 17:18 UTC (permalink / raw) To: Grant Rettke, Nicolas Goaziou, Org-mode Hi Grant, 2014ko irailak 23an, Grant Rettke-ek idatzi zuen: > > Aaron may I trouble you to add a flag so that if such errors occur > then the export fails? > > From my perspective, if the document doesn't "compile", then nothing > should succeed. > > Compile includes export from my perspective. Indeed, that’s the direction that the next iteration of this patch will move, motivated by your and Nicolas’s comments. Thanks, -- Aaron Ecay ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 17:18 ` Aaron Ecay @ 2014-09-23 17:29 ` Jacob Gerlach 2014-09-23 21:10 ` Nicolas Goaziou 0 siblings, 1 reply; 21+ messages in thread From: Jacob Gerlach @ 2014-09-23 17:29 UTC (permalink / raw) To: Org-mode [-- Attachment #1: Type: text/plain, Size: 1066 bytes --] While I expect that there's no code overlap with macro expansion, I asked about similar "stop on unresolved content" behavior with respect to links here: http://article.gmane.org/gmane.emacs.orgmode/90010/ That discussion died off so I thought I'd bring up the topic again here. Would it be possible to cause export to stop (configurably, as discussed) when a link can't be resolved instead of just applying a special format (i.e. org-latex-link-with-unknown-path-format) On Tue, Sep 23, 2014 at 1:18 PM, Aaron Ecay <aaronecay@gmail.com> wrote: > Hi Grant, > > 2014ko irailak 23an, Grant Rettke-ek idatzi zuen: > > > > Aaron may I trouble you to add a flag so that if such errors occur > > then the export fails? > > > > From my perspective, if the document doesn't "compile", then nothing > > should succeed. > > > > Compile includes export from my perspective. > > Indeed, that’s the direction that the next iteration of this patch will > move, motivated by your and Nicolas’s comments. > > Thanks, > > -- > Aaron Ecay > > [-- Attachment #2: Type: text/html, Size: 1645 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 17:29 ` Jacob Gerlach @ 2014-09-23 21:10 ` Nicolas Goaziou 0 siblings, 0 replies; 21+ messages in thread From: Nicolas Goaziou @ 2014-09-23 21:10 UTC (permalink / raw) To: Jacob Gerlach; +Cc: Org-mode Hello, Jacob Gerlach <jacobgerlach@gmail.com> writes: > While I expect that there's no code overlap with macro expansion, I asked > about similar "stop on unresolved content" behavior with respect to links > here: http://article.gmane.org/gmane.emacs.orgmode/90010/ > > That discussion died off so I thought I'd bring up the topic again here. > > Would it be possible to cause export to stop (configurably, as discussed) > when a link can't be resolved instead of just applying a special format > (i.e. org-latex-link-with-unknown-path-format) I have no objection to return an error when a fuzzy link, and id link or a custom id link cannot be matched. However, I do not like the idea of configurability as it would make the code a bit more complex for little gain. Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 3:25 ` Aaron Ecay 2014-09-23 16:59 ` Grant Rettke @ 2014-09-23 20:24 ` Nicolas Goaziou 2014-09-28 3:53 ` Aaron Ecay 2014-09-23 20:26 ` Rasmus 2 siblings, 1 reply; 21+ messages in thread From: Nicolas Goaziou @ 2014-09-23 20:24 UTC (permalink / raw) To: Org-mode Hello, Aaron Ecay <aaronecay@gmail.com> writes: > Um...but the patch I sent works precisely by defining a macro translator, > which does get called for any unexpanded (because undefined) macros. Macros are not expected to be seen by export back-ends. It happens when a macro is undefined, but this is not a reliable feature. > I guess you’re saying the code ought to block this at a lower/earlier > level. Yes, at "org-macro.el" level. > I think error is better than obnoxious message, because it’s possible > for the latter to slip through into a “production” document. (We ought > to proofread our documents carefully, of course...but no one’s > perfect). Sounds good. > One issue is that the exporter does two macro expansion passes – one > for garden-variety macros, and the second for author, date, email, and > title. So, we can’t make the macro expansion unconditionally barf on > undefined macros (since for the first pass e.g. author is undefined). > I see three options: > 1. explicitly whitelist the few “blessed” macros like author, and error > on any other undefined macro > 2. add an optional “final” arg to org-macro-replace-all, which will > activate the undefined checking only if non-nil, and pass this flag > in the exporter’s second (and last) call to org-macro-replace-all > 3. in ‘org-export-as’, manually walk the parse tree after expanding > macros, and make sure no 'macro type objects are left > > WDYT? I have no strong opinion here but I lean towards option 2 as the error stays internal to "org-macro.el" and is only triggered with an optional argument. It also doesn't require to hardcode special macro names. Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 20:24 ` Nicolas Goaziou @ 2014-09-28 3:53 ` Aaron Ecay 2014-09-28 6:59 ` Nicolas Goaziou 0 siblings, 1 reply; 21+ messages in thread From: Aaron Ecay @ 2014-09-28 3:53 UTC (permalink / raw) To: Nicolas Goaziou, Org-mode [-- Attachment #1: Type: text/plain, Size: 1918 bytes --] Hi Nicolas, Thanks for the feedback. 2014ko irailak 23an, Nicolas Goaziou-ek idatzi zuen: > > Hello, > > Aaron Ecay <aaronecay@gmail.com> writes: > >> Um...but the patch I sent works precisely by defining a macro translator, >> which does get called for any unexpanded (because undefined) macros. > > Macros are not expected to be seen by export back-ends. It happens when > a macro is undefined, but this is not a reliable feature. > >> I guess you’re saying the code ought to block this at a lower/earlier >> level. > > Yes, at "org-macro.el" level. > >> I think error is better than obnoxious message, because it’s possible >> for the latter to slip through into a “production” document. (We ought >> to proofread our documents carefully, of course...but no one’s >> perfect). > > Sounds good. > >> One issue is that the exporter does two macro expansion passes – one >> for garden-variety macros, and the second for author, date, email, and >> title. So, we can’t make the macro expansion unconditionally barf on >> undefined macros (since for the first pass e.g. author is undefined). >> I see three options: >> 1. explicitly whitelist the few “blessed” macros like author, and error >> on any other undefined macro >> 2. add an optional “final” arg to org-macro-replace-all, which will >> activate the undefined checking only if non-nil, and pass this flag >> in the exporter’s second (and last) call to org-macro-replace-all >> 3. in ‘org-export-as’, manually walk the parse tree after expanding >> macros, and make sure no 'macro type objects are left >> >> WDYT? > > I have no strong opinion here but I lean towards option 2 as the error > stays internal to "org-macro.el" and is only triggered with an optional > argument. It also doesn't require to hardcode special macro names. Attached is a revised patch. WDYT? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Warn-about-unexpanded-macros-on-export.patch --] [-- Type: text/x-diff, Size: 2983 bytes --] From e386809601d99291885136de1d9be84193b69a2c Mon Sep 17 00:00:00 2001 From: Aaron Ecay <aaronecay@gmail.com> Date: Sat, 27 Sep 2014 23:50:23 -0400 Subject: [PATCH] Warn about unexpanded macros on export * lisp/org-macro.el (org-macro-replace-all): Add optional `finalize' argument. * lisp/ox.el (org-export-as): Use it. --- lisp/org-macro.el | 35 ++++++++++++++++++++++------------- lisp/ox.el | 3 ++- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/lisp/org-macro.el b/lisp/org-macro.el index 50ce438..307e232 100644 --- a/lisp/org-macro.el +++ b/lisp/org-macro.el @@ -155,10 +155,14 @@ default value. Return nil if no template was found." ;; Return string. (format "%s" (or value "")))))) -(defun org-macro-replace-all (templates) +(defun org-macro-replace-all (templates &optional finalize) "Replace all macros in current buffer by their expansion. + TEMPLATES is an alist of templates used for expansion. See -`org-macro-templates' for a buffer-local default value." +`org-macro-templates' for a buffer-local default value. + +If optional arg FINALIZE is non-nil, raise an error if a macro is +found in the buffer with no definition in TEMPLATES." (save-excursion (goto-char (point-min)) (let (record) @@ -176,17 +180,22 @@ TEMPLATES is an alist of templates used for expansion. See (if (member signature record) (error "Circular macro expansion: %s" (org-element-property :key object)) - (when value - (push signature record) - (delete-region - begin - ;; Preserve white spaces after the macro. - (progn (goto-char (org-element-property :end object)) - (skip-chars-backward " \t") - (point))) - ;; Leave point before replacement in case of recursive - ;; expansions. - (save-excursion (insert value))))))))))) + (if value + (progn + (push signature record) + (delete-region + begin + ;; Preserve white spaces after the macro. + (progn (goto-char (org-element-property :end object)) + (skip-chars-backward " \t") + (point))) + ;; Leave point before replacement in case of recursive + ;; expansions. + (save-excursion (insert value))) + (when finalize + (error "Macro %s was undefined at line %s" + (org-element-property :key object) + (line-number-at-pos)))))))))))) (provide 'org-macro) diff --git a/lisp/ox.el b/lisp/ox.el index 59091fc..216a375 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3136,7 +3136,8 @@ Return code as a string." ;; EMAIL is not a parsed keyword: store it as-is. (cons "email" (or (plist-get info :email) "")) (cons "title" - (org-element-interpret-data (plist-get info :title))))) + (org-element-interpret-data (plist-get info :title)))) + 'finalize) ;; Parse buffer. (setq tree (org-element-parse-buffer nil visible-only)) ;; Handle left-over uninterpreted elements or objects in -- 2.1.1 [-- Attachment #3: Type: text/plain, Size: 15 bytes --] -- Aaron Ecay ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-28 3:53 ` Aaron Ecay @ 2014-09-28 6:59 ` Nicolas Goaziou 2014-10-12 15:48 ` Aaron Ecay 0 siblings, 1 reply; 21+ messages in thread From: Nicolas Goaziou @ 2014-09-28 6:59 UTC (permalink / raw) To: Org-mode Hello, Aaron Ecay <aaronecay@gmail.com> writes: > Attached is a revised patch. WDYT? Looks good. Some small comments follow. > + (if value > + (progn > + (push signature record) > + (delete-region > + begin > + ;; Preserve white spaces after the macro. > + (progn (goto-char (org-element-property :end object)) > + (skip-chars-backward " \t") > + (point))) > + ;; Leave point before replacement in case of recursive > + ;; expansions. > + (save-excursion (insert value))) > + (when finalize > + (error "Macro %s was undefined at line %s" > + (org-element-property :key object) > + (line-number-at-pos)))))))))))) Nitpick: I find the following more readable (cond (value (push signature record) ...) (finalize (error ...))) Also, don't provide error line as macro are replaced after include keywords are expanded. IOW, in some cases, the line number will be misleading. The key is sufficient, e.g., (error "Undefined Org macro: %s. Aborting" (org-element-property :key object)) You can commit it once this is fixed. Thank you for the patch. Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-28 6:59 ` Nicolas Goaziou @ 2014-10-12 15:48 ` Aaron Ecay 0 siblings, 0 replies; 21+ messages in thread From: Aaron Ecay @ 2014-10-12 15:48 UTC (permalink / raw) To: Nicolas Goaziou, Org-mode Hi Nicolas, 2014ko irailak 28an, Nicolas Goaziou-ek idatzi zuen: > > Hello, > > Aaron Ecay <aaronecay@gmail.com> writes: > >> Attached is a revised patch. WDYT? > > Looks good. Some small comments follow. > >> + (if value >> + (progn >> + (push signature record) >> + (delete-region >> + begin >> + ;; Preserve white spaces after the macro. >> + (progn (goto-char (org-element-property :end object)) >> + (skip-chars-backward " \t") >> + (point))) >> + ;; Leave point before replacement in case of recursive >> + ;; expansions. >> + (save-excursion (insert value))) >> + (when finalize >> + (error "Macro %s was undefined at line %s" >> + (org-element-property :key object) >> + (line-number-at-pos)))))))))))) > > Nitpick: I find the following more readable > > (cond (value (push signature record) > ...) > (finalize (error ...))) > > Also, don't provide error line as macro are replaced after include > keywords are expanded. IOW, in some cases, the line number will be > misleading. The key is sufficient, e.g., > > (error "Undefined Org macro: %s. Aborting" > (org-element-property :key object)) > > You can commit it once this is fixed. Thank you for the patch. Pushed; thanks for the feedback. -- Aaron Ecay ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 3:25 ` Aaron Ecay 2014-09-23 16:59 ` Grant Rettke 2014-09-23 20:24 ` Nicolas Goaziou @ 2014-09-23 20:26 ` Rasmus 2014-09-28 4:00 ` Aaron Ecay 2 siblings, 1 reply; 21+ messages in thread From: Rasmus @ 2014-09-23 20:26 UTC (permalink / raw) To: emacs-orgmode Hi, Aaron Ecay <aaronecay@gmail.com> writes: >> You have two options. Either report an error, as you suggested, or >> insert an obnoxious message in the output, e.g., "UNKNOWN MACRO", à la >> "DEFINITION NOT FOUND." for footnote definitions. In any case, this >> should happen in "org-macro.el", not in the export framework. This is pretty annoying for footnotes. > I think error is better than obnoxious message, because it’s possible > for the latter to slip through into a “production” document. (We ought > to proofread our documents carefully, of course...but no one’s perfect). I think an error is correct, but the message should be informative, e.g. "macro MACRO undefined at LINE". —Rasmus -- Summon the Mothership! ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-23 20:26 ` Rasmus @ 2014-09-28 4:00 ` Aaron Ecay 2014-09-28 7:03 ` Nicolas Goaziou 0 siblings, 1 reply; 21+ messages in thread From: Aaron Ecay @ 2014-09-28 4:00 UTC (permalink / raw) To: Rasmus, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 907 bytes --] Hi Rasmus, 2014ko irailak 23an, Rasmus-ek idatzi zuen: > > Hi, > > Aaron Ecay <aaronecay@gmail.com> writes: > >>> You have two options. Either report an error, as you suggested, or >>> insert an obnoxious message in the output, e.g., "UNKNOWN MACRO", à la >>> "DEFINITION NOT FOUND." for footnote definitions. In any case, this >>> should happen in "org-macro.el", not in the export framework. > > This is pretty annoying for footnotes. This turned out to be very easy to change; attached is a patch. The links issue (re-)raised by Jacob in <http://mid.gmane.org/CAA6UvuFm-1nWD06A5o3HwSvEQmgqLJNcfK_PtoHSEhcVdJTYQw@mail.gmail.com> is a bit harder to deal with, since each backend currently does something a little different. It would be possible to make every backend’s org-X-link function error out at the end, but I’m not sure if that’s the right thing to do or not. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-export-Raise-an-error-if-footnote-definition-is-not-.patch --] [-- Type: text/x-diff, Size: 1285 bytes --] From 123d5fcd4f4594c04210ab873395e7def8790450 Mon Sep 17 00:00:00 2001 From: Aaron Ecay <aaronecay@gmail.com> Date: Sat, 27 Sep 2014 23:57:01 -0400 Subject: [PATCH 2/2] [export] Raise an error if footnote definition is not found. * lisp/ox.el (org-export-get-footnote-definition): Raise an error if footnote definition is not found. --- lisp/ox.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 216a375..2516a22 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -3711,12 +3711,12 @@ INFO is the plist used as a communication channel." (defun org-export-get-footnote-definition (footnote-reference info) "Return definition of FOOTNOTE-REFERENCE as parsed data. INFO is the plist used as a communication channel. If no such -definition can be found, return \"DEFINITION NOT FOUND\"." +definition can be found, raise an error." (let ((label (org-element-property :label footnote-reference))) (or (if label (cdr (assoc label (plist-get info :footnote-definition-alist))) (org-element-contents footnote-reference)) - "DEFINITION NOT FOUND."))) + (error "Definition not found for footnote %s" label)))) (defun org-export-get-footnote-number (footnote info) "Return number associated to a footnote. -- 2.1.1 [-- Attachment #3: Type: text/plain, Size: 15 bytes --] -- Aaron Ecay ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-28 4:00 ` Aaron Ecay @ 2014-09-28 7:03 ` Nicolas Goaziou 2014-10-12 15:49 ` Aaron Ecay 2015-03-12 2:55 ` Jacob Gerlach 0 siblings, 2 replies; 21+ messages in thread From: Nicolas Goaziou @ 2014-09-28 7:03 UTC (permalink / raw) To: Rasmus; +Cc: emacs-orgmode Hello, Aaron Ecay <aaronecay@gmail.com> writes: > This turned out to be very easy to change; attached is a patch. Thank you. Please apply it. > The links issue (re-)raised by Jacob in > <http://mid.gmane.org/CAA6UvuFm-1nWD06A5o3HwSvEQmgqLJNcfK_PtoHSEhcVdJTYQw@mail.gmail.com> > is a bit harder to deal with, since each backend currently does > something a little different. It would be possible to make every > backend’s org-X-link function error out at the end, but I’m not sure > if that’s the right thing to do or not. A more workable solution would be to focus on internal links only and patch `org-export-resolve-id-link', `org-export-resolve-fuzzy-link', `org-export-resolve-coderef' (not needed for radio links). Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-28 7:03 ` Nicolas Goaziou @ 2014-10-12 15:49 ` Aaron Ecay 2014-10-12 17:19 ` Nicolas Goaziou 2015-03-12 2:55 ` Jacob Gerlach 1 sibling, 1 reply; 21+ messages in thread From: Aaron Ecay @ 2014-10-12 15:49 UTC (permalink / raw) To: Nicolas Goaziou, Rasmus; +Cc: emacs-orgmode 2014ko irailak 28an, Nicolas Goaziou-ek idatzi zuen: > > Hello, > > Aaron Ecay <aaronecay@gmail.com> writes: > >> This turned out to be very easy to change; attached is a patch. > > Thank you. Please apply it. Pushed. -- Aaron Ecay ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-10-12 15:49 ` Aaron Ecay @ 2014-10-12 17:19 ` Nicolas Goaziou 0 siblings, 0 replies; 21+ messages in thread From: Nicolas Goaziou @ 2014-10-12 17:19 UTC (permalink / raw) To: Rasmus; +Cc: emacs-orgmode Hello, Aaron Ecay <aaronecay@gmail.com> writes: > Pushed. Thanks. -- Nicolas Goaziou 0x80A93738 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2014-09-28 7:03 ` Nicolas Goaziou 2014-10-12 15:49 ` Aaron Ecay @ 2015-03-12 2:55 ` Jacob Gerlach 2015-03-15 8:44 ` Nicolas Goaziou 1 sibling, 1 reply; 21+ messages in thread From: Jacob Gerlach @ 2015-03-12 2:55 UTC (permalink / raw) To: Org-mode [-- Attachment #1: Type: text/plain, Size: 2096 bytes --] On Sun, Sep 28, 2014 at 3:03 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > Aaron Ecay <aaronecay@gmail.com> writes: >> The links issue (re-)raised by Jacob in >> <http://mid.gmane.org/CAA6UvuFm-1nWD06A5o3HwSvEQmgqLJNcfK_PtoHSEhcVdJTYQw@mail.gmail.com> >> is a bit harder to deal with, since each backend currently does >> something a little different. It would be possible to make every >> backend’s org-X-link function error out at the end, but I’m not sure >> if that’s the right thing to do or not. > > A more workable solution would be to focus on internal links only and > patch `org-export-resolve-id-link', `org-export-resolve-fuzzy-link', > `org-export-resolve-coderef' (not needed for radio links). Patch attached for fuzzy links. I don't really use code blocks, so I wasn't sure what to do with org-export-resolve-coderef. Should the final (when (re-search... (cond ... become (or (re-search... (cond ... (error ID links are tricky. AFAICT, an invalid id link will always resolve to the file it's contained in. The last check in org-export-resolve-id-link is looking for the id in `:id-alist'. Is this the same `:id-alist' built in org-export-get-environment? If so, is it desirable for org-id-find-id-file to fall back on the current buffer (the current behavior)? It seems like this will mistakenly cause org-export-get-environment to think that the bad link is valid and pointing to an "external" file. IIUC, the fall back behavior doesn't occur inside org-test-with-parsed-data because buffer-file-name returns nil. I don't know the implications of changing org-id-find-id-file. The fall back behavior was introduced in ac83bc01 when org-id was mostly rewritten. Removing the fall back behavior doesn't cause any failures on `make test'. If it's acceptable to remove the fall back, I can provide a similar patch for org-export-resolve-id-link. Example, foo.org: [[id:points-nowhere]] [[id:this-one-too][also bad]] Exports (latex) to: \url{foo.org} \href{foo.org}{also bad} Regards, Jake [-- Attachment #2: 0001-ox.el-Issue-error-for-unresolved-fuzzy-link.patch --] [-- Type: text/x-patch, Size: 2255 bytes --] From 20f84420a84997fc0e15df5af2e65b3cfde87ac1 Mon Sep 17 00:00:00 2001 From: Jacob Gerlach <jacobgerlach@gmail.com> Date: Wed, 11 Mar 2015 22:39:11 -0400 Subject: [PATCH] ox.el: Issue error for unresolved fuzzy link * lisp/org-capture.el (org-export-resolve-fuzzy-link): throw error instead of returning nil when link can't be resolved. * testing/lisp/test-ox.el (test-org-export/resolve-fuzzy-link): change last test from should-not to should-error In addition to throwing an error, don't store the failed match in the link cache. TINYCHANGE --- lisp/ox.el | 7 ++++--- testing/lisp/test-ox.el | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 0c7728f..a28a227 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -4068,7 +4068,7 @@ significant." ;; Last case: link either points to a headline or to nothingness. ;; Try to find the source, with priority given to headlines with ;; the closest common ancestor. If such candidate is found, - ;; return it, otherwise return nil. + ;; return it, otherwise signal an error. (t (let ((find-headline (function @@ -4094,8 +4094,9 @@ significant." (org-element-lineage parent-hl nil t)))) (let ((foundp (funcall find-headline path parent))) (when foundp (throw 'exit foundp)))) - ;; No destination found: return nil. - (and (not match-title-p) (puthash path nil link-cache)))))))) + ;; No destination found: error. + (unless match-title-p + (error (format "Unable to resolve link \"%s\"" raw-path))))))))) (defun org-export-resolve-id-link (link info) "Return headline referenced as LINK destination. diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 1b70a78..7cf1e1d 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -2478,8 +2478,8 @@ Another text. (ref:text) (org-element-type (org-export-resolve-fuzzy-link (org-element-map tree 'link 'identity info t) info))))) - ;; Return nil if no match. - (should-not + ;; Error if no match. + (should-error (org-test-with-parsed-data "[[target]]" (org-export-resolve-fuzzy-link (org-element-map tree 'link 'identity info t) info))) -- 1.9.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2015-03-12 2:55 ` Jacob Gerlach @ 2015-03-15 8:44 ` Nicolas Goaziou 2015-03-17 16:20 ` Jacob Gerlach 0 siblings, 1 reply; 21+ messages in thread From: Nicolas Goaziou @ 2015-03-15 8:44 UTC (permalink / raw) To: Jacob Gerlach; +Cc: Org-mode Jacob Gerlach <jacobgerlach@gmail.com> writes: > Patch attached for fuzzy links. Thank you. > I don't really use code blocks, so I wasn't sure what to do with > org-export-resolve-coderef. Should the final > > (when (re-search... > (cond ... > > become > > (or (re-search... > (cond ... > (error No, the whole body could be wrapped with an `or': (or (org-element-map ...) (user-error "Unable to resolve code reference: %s" ref)) > ID links are tricky. AFAICT, an invalid id link will always resolve to > the file it's contained in. The last check in > org-export-resolve-id-link is looking for the id in `:id-alist'. Is > this the same `:id-alist' built in org-export-get-environment? Yes, it is. > If so, is it desirable for org-id-find-id-file to fall back on the > current buffer (the current behavior)? According to its docstring, `org-id-find-id-file' returns nil when search failed. Isn't it the case? > I don't know the implications of changing org-id-find-id-file. The > fall back behavior was introduced in ac83bc01 when org-id was mostly > rewritten. Removing the fall back behavior doesn't cause any failures > on `make test'. If it's acceptable to remove the fall back, I can > provide a similar patch for org-export-resolve-id-link. `org-export-resolve-id-link' could throw an error, indeed. > Subject: [PATCH] ox.el: Issue error for unresolved fuzzy link > > * lisp/org-capture.el (org-export-resolve-fuzzy-link): throw error ^^^^^^^^^^^ wrong file You need to capitalize sentence: "Throw an error" > instead of returning nil when link can't be resolved. > > * testing/lisp/test-ox.el (test-org-export/resolve-fuzzy-link): change > last test from should-not to should-error Ditto. > + ;; No destination found: error. > + (unless match-title-p > + (error (format "Unable to resolve link \"%s\"" raw-path))))))))) You don't need to check `match-title-p' here. Also, it should be `user-error' instead of `error'. Regards, ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2015-03-15 8:44 ` Nicolas Goaziou @ 2015-03-17 16:20 ` Jacob Gerlach 2015-03-17 22:38 ` Nicolas Goaziou 0 siblings, 1 reply; 21+ messages in thread From: Jacob Gerlach @ 2015-03-17 16:20 UTC (permalink / raw) To: Org-mode [-- Attachment #1: Type: text/plain, Size: 1975 bytes --] Hello, Thanks for the feedback. On Sun, Mar 15, 2015 at 4:44 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: >> I don't really use code blocks, so I wasn't sure what to do with >> org-export-resolve-coderef. [...] > No, the whole body could be wrapped with an `or': > > (or (org-element-map ...) > (user-error "Unable to resolve code reference: %s" ref)) I tried this, or more specifically: (or ((org-element-map ... info 'first-match)) (user-error ... and got a failure on test-org-export/resolve-coderef. It's not obvious to me from reading the tests if there is a test that needs to be changed, or if it's a legitimate failure and a different approach is needed (or if I made a mistake). I asked in [1] for some guidance on tests, and I'm still lost. >> If so, is it desirable for org-id-find-id-file to fall back on the >> current buffer (the current behavior)? > > According to its docstring, `org-id-find-id-file' returns nil when > search failed. Isn't it the case? Are you looking at `org-id-find-id-in-file' rather than `org-id-find-id-file'? The docstring for `org-id-find-file' only says: "Query the id database for the file in which this ID is located." [...] > `org-export-resolve-id-link' could throw an error, indeed. I'm not clear on the way forward for id links. I propose removing the fall back behavior in `org-id-find-id-file'. If that's acceptable, I can provide a patch for `org-export-resolve-id-link'. >> Subject: [PATCH] ox.el: Issue error for unresolved fuzzy link [...] Commit message fixed. I also updated the docstring for org-export-resolve-fuzzy link. >> + ;; No destination found: error. >> + (unless match-title-p >> + (error (format "Unable to resolve link \"%s\"" raw-path))))))))) > > You don't need to check `match-title-p' here. Also, it should be > `user-error' instead of `error'. OK. Updated fuzzy link patch attached. Regards, Jake [1] http://thread.gmane.org/gmane.emacs.orgmode/96017 [-- Attachment #2: 0001-ox.el-Issue-error-for-unresolved-fuzzy-link.patch --] [-- Type: text/x-patch, Size: 2550 bytes --] From a9221500b721a501f7b7f05519dfc0d6f30f23d0 Mon Sep 17 00:00:00 2001 From: Jacob Gerlach <jacobgerlach@gmail.com> Date: Wed, 11 Mar 2015 22:39:11 -0400 Subject: [PATCH] ox.el: Issue error for unresolved fuzzy link * lisp/ox.el (org-export-resolve-fuzzy-link): Throw an error instead of returning nil when link can't be resolved. * testing/lisp/test-ox.el (test-org-export/resolve-fuzzy-link): Change last test from should-not to should-error In addition to throwing an error, don't store the failed match in the link cache. TINYCHANGE --- lisp/ox.el | 8 ++++---- testing/lisp/test-ox.el | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lisp/ox.el b/lisp/ox.el index 0c7728f..9461117 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -4021,7 +4021,7 @@ Return value can be an object, an element, or nil: will be given to the one with the closest common ancestor, if any, or the first one in the parse tree otherwise. -- Otherwise, return nil. +- Otherwise, report an error. Assume LINK type is \"fuzzy\". White spaces are not significant." @@ -4068,7 +4068,7 @@ significant." ;; Last case: link either points to a headline or to nothingness. ;; Try to find the source, with priority given to headlines with ;; the closest common ancestor. If such candidate is found, - ;; return it, otherwise return nil. + ;; return it, otherwise signal an error. (t (let ((find-headline (function @@ -4094,8 +4094,8 @@ significant." (org-element-lineage parent-hl nil t)))) (let ((foundp (funcall find-headline path parent))) (when foundp (throw 'exit foundp)))) - ;; No destination found: return nil. - (and (not match-title-p) (puthash path nil link-cache)))))))) + ;; No destination found: error. + (user-error (format "Unable to resolve link \"%s\"" raw-path)))))))) (defun org-export-resolve-id-link (link info) "Return headline referenced as LINK destination. diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index 1b70a78..7cf1e1d 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -2478,8 +2478,8 @@ Another text. (ref:text) (org-element-type (org-export-resolve-fuzzy-link (org-element-map tree 'link 'identity info t) info))))) - ;; Return nil if no match. - (should-not + ;; Error if no match. + (should-error (org-test-with-parsed-data "[[target]]" (org-export-resolve-fuzzy-link (org-element-map tree 'link 'identity info t) info))) -- 1.9.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2015-03-17 16:20 ` Jacob Gerlach @ 2015-03-17 22:38 ` Nicolas Goaziou 2015-03-18 20:25 ` Jacob Gerlach 0 siblings, 1 reply; 21+ messages in thread From: Nicolas Goaziou @ 2015-03-17 22:38 UTC (permalink / raw) To: Jacob Gerlach; +Cc: Org-mode Jacob Gerlach <jacobgerlach@gmail.com> writes: > I tried this, or more specifically: > (or ((org-element-map > ... > info 'first-match)) > (user-error ... > and got a failure on test-org-export/resolve-coderef. It's not > obvious to me from reading the tests if there is a test that needs to > be changed, or if it's a legitimate failure and a different approach > is needed (or if I made a mistake). I asked in [1] for some guidance > on tests, and I'm still lost. There is a spurious (...) in the code above. I implemented it in a886b234763f288670103d2c98169164dedec06c. >> According to its docstring, `org-id-find-id-file' returns nil when >> search failed. Isn't it the case? > Are you looking at `org-id-find-id-in-file' rather than > `org-id-find-id-file'? The docstring for `org-id-find-file' only says: > "Query the id database for the file in which this ID is located." You're right. >> `org-export-resolve-id-link' could throw an error, indeed. > I'm not clear on the way forward for id links. I propose removing the > fall back behavior in `org-id-find-id-file'. If that's acceptable, I > can provide a patch for `org-export-resolve-id-link'. Actually, we don't even need to do that. I changed "ox.el" so it uses `org-id-find' instead of `org-id-find-id-file'. The former has a better fall-back value. See e0b19dedb99d3c01199a159a2da8f9aa4adb2f6f > Updated fuzzy link patch attached. Applied (with minor tweaks). Thank you. Regards, ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC] [PATCH] Warn about unexpanded macros on export 2015-03-17 22:38 ` Nicolas Goaziou @ 2015-03-18 20:25 ` Jacob Gerlach 0 siblings, 0 replies; 21+ messages in thread From: Jacob Gerlach @ 2015-03-18 20:25 UTC (permalink / raw) To: Jacob Gerlach, Org-mode On Tue, Mar 17, 2015 at 6:38 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: (re unresolved ID links) >See e0b19dedb99d3c01199a159a2da8f9aa4adb2f6f >> Updated fuzzy link patch attached. > Applied (with minor tweaks). Thank you. Thanks for the changes. I had some issues using the error messages to track down bad links with descriptions since the link path is hidden. (isearch doesn't search the hidden text.) If anyone else is similarly challenged, `org-toggle-link-display' is very helpful here. Regards, Jake ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-03-18 20:25 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-19 19:12 [RFC] [PATCH] Warn about unexpanded macros on export Aaron Ecay 2014-09-19 19:29 ` Nicolas Goaziou 2014-09-23 3:25 ` Aaron Ecay 2014-09-23 16:59 ` Grant Rettke 2014-09-23 17:18 ` Aaron Ecay 2014-09-23 17:29 ` Jacob Gerlach 2014-09-23 21:10 ` Nicolas Goaziou 2014-09-23 20:24 ` Nicolas Goaziou 2014-09-28 3:53 ` Aaron Ecay 2014-09-28 6:59 ` Nicolas Goaziou 2014-10-12 15:48 ` Aaron Ecay 2014-09-23 20:26 ` Rasmus 2014-09-28 4:00 ` Aaron Ecay 2014-09-28 7:03 ` Nicolas Goaziou 2014-10-12 15:49 ` Aaron Ecay 2014-10-12 17:19 ` Nicolas Goaziou 2015-03-12 2:55 ` Jacob Gerlach 2015-03-15 8:44 ` Nicolas Goaziou 2015-03-17 16:20 ` Jacob Gerlach 2015-03-17 22:38 ` Nicolas Goaziou 2015-03-18 20:25 ` Jacob Gerlach
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).