emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Improve message when file to include is missing
@ 2014-01-24 10:49 Sebastien Vauban
  2014-01-24 11:14 ` Nicolas Goaziou
  2014-01-24 11:15 ` Bastien
  0 siblings, 2 replies; 8+ messages in thread
From: Sebastien Vauban @ 2014-01-24 10:49 UTC (permalink / raw)
  To: emacs-orgmode-mXXj517/zsQ

Hello,

When a SETUPFILE is missing, there is an error or message generated, but
we don't know in which file the bad reference is -- when all those files
are loaded during the agenda generation.

Hence, a better message, specifying where to go and look for the bad
link.

Best regards,
  Seb

From 4c31c1cf649c2eb3487e61e56296f058a62d7c08 Mon Sep 17 00:00:00 2001
From: "Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
Date: Fri, 24 Jan 2014 11:43:52 +0100
Subject: [PATCH] Improve message when file to include is missing

* org.el (org-file-contents): Improve message when linked file does not exist.

---
 lisp/org.el |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index a225bae..787a09f 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5235,8 +5235,10 @@ Support for group tags is controlled by the option
   (if (or (not file)
 	  (not (file-readable-p file)))
       (if noerror
-	  (message "Cannot read file \"%s\"" file)
-	(error "Cannot read file \"%s\"" file))
+	  (message "Cannot read file \"%s\" referenced in file \"%s\""
+		   file (buffer-file-name))
+	(error "Cannot read file \"%s\" referenced in file \"%s\""
+	       file (buffer-file-name)))
     (with-temp-buffer
       (insert-file-contents file)
       (buffer-string))))
-- 
1.7.9

-- 
Sebastien Vauban

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-01-24 10:49 [PATCH] Improve message when file to include is missing Sebastien Vauban
@ 2014-01-24 11:14 ` Nicolas Goaziou
  2014-01-24 11:19   ` Bastien
  2014-02-18 14:55   ` Sebastien Vauban
  2014-01-24 11:15 ` Bastien
  1 sibling, 2 replies; 8+ messages in thread
From: Nicolas Goaziou @ 2014-01-24 11:14 UTC (permalink / raw)
  To: Sebastien Vauban; +Cc: public-emacs-orgmode-mXXj517/zsQ



Hello,

"Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> When a SETUPFILE is missing, there is an error or message generated, but
> we don't know in which file the bad reference is -- when all those files
> are loaded during the agenda generation.
>
> Hence, a better message, specifying where to go and look for the bad
> link.

Thanks.

> From 4c31c1cf649c2eb3487e61e56296f058a62d7c08 Mon Sep 17 00:00:00 2001
> From: "Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
> Date: Fri, 24 Jan 2014 11:43:52 +0100
> Subject: [PATCH] Improve message when file to include is missing
>
> * org.el (org-file-contents): Improve message when linked file does not exist.
>
> ---
>  lisp/org.el |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index a225bae..787a09f 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -5235,8 +5235,10 @@ Support for group tags is controlled by the option
>    (if (or (not file)
>  	  (not (file-readable-p file)))
>        (if noerror
> -	  (message "Cannot read file \"%s\"" file)
> -	(error "Cannot read file \"%s\"" file))
> +	  (message "Cannot read file \"%s\" referenced in file \"%s\""
> +		   file (buffer-file-name))
> +	(error "Cannot read file \"%s\" referenced in file \"%s\""
> +	       file (buffer-file-name)))

What if `buffer-file-name' returns nil? Sure, the problem won't happen
during agenda generation, but `org-file-contents' is used elsewhere.

Also, it is better to use:

  (buffer-file-name (buffer-base-buffer))

since the current buffer may be an indirect one.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-01-24 10:49 [PATCH] Improve message when file to include is missing Sebastien Vauban
  2014-01-24 11:14 ` Nicolas Goaziou
@ 2014-01-24 11:15 ` Bastien
  1 sibling, 0 replies; 8+ messages in thread
From: Bastien @ 2014-01-24 11:15 UTC (permalink / raw)
  To: Sebastien Vauban; +Cc: public-emacs-orgmode-mXXj517/zsQ



"Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> When a SETUPFILE is missing, there is an error or message generated, but
> we don't know in which file the bad reference is -- when all those files
> are loaded during the agenda generation.
>
> Hence, a better message, specifying where to go and look for the bad
> link.

Applied in master, thanks!

-- 
 Bastien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-01-24 11:14 ` Nicolas Goaziou
@ 2014-01-24 11:19   ` Bastien
  2014-02-18 14:55   ` Sebastien Vauban
  1 sibling, 0 replies; 8+ messages in thread
From: Bastien @ 2014-01-24 11:19 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: public-emacs-orgmode-mXXj517/zsQ, Sebastien Vauban



Nicolas Goaziou <n.goaziou@gmail.com> writes:

> What if `buffer-file-name' returns nil? Sure, the problem won't happen
> during agenda generation, but `org-file-contents' is used elsewhere.
>
> Also, it is better to use:
>
>   (buffer-file-name (buffer-base-buffer))
>
> since the current buffer may be an indirect one.

I reverted the commit -- I let you guys discuss it and provide the
correct one.

Thanks,

-- 
 Bastien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-01-24 11:14 ` Nicolas Goaziou
  2014-01-24 11:19   ` Bastien
@ 2014-02-18 14:55   ` Sebastien Vauban
  2014-02-18 20:21     ` Nicolas Goaziou
  1 sibling, 1 reply; 8+ messages in thread
From: Sebastien Vauban @ 2014-02-18 14:55 UTC (permalink / raw)
  To: emacs-orgmode-mXXj517/zsQ

Hello Nicolas,

Nicolas Goaziou wrote:
> "Sebastien Vauban" writes:
>
>> When a SETUPFILE is missing, there is an error or message generated, but
>> we don't know in which file the bad reference is -- when all those files
>> are loaded during the agenda generation.
>>
>> Hence, a better message, specifying where to go and look for the bad
>> link.
>
> What if `buffer-file-name' returns nil? Sure, the problem won't happen
> during agenda generation, but `org-file-contents' is used elsewhere.
>
> Also, it is better to use:
>
>   (buffer-file-name (buffer-base-buffer))
>
> since the current buffer may be an indirect one.

This should answer your (fruitful) comments.

Best regards,
  Seb

-- 
Sebastien Vauban


From 804fff53730f0da2e1b41b7b9f070e8e23c8974b Mon Sep 17 00:00:00 2001
From: "Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
Date: Tue, 18 Feb 2014 15:49:57 +0100
Subject: [PATCH] Improve message when file to include is missing

* org.el (org-file-contents): Improve message when linked file does not exist.

---
 lisp/org.el |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index dfb0517..138e735 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5230,14 +5230,18 @@ Support for group tags is controlled by the option
 
 (defun org-file-contents (file &optional noerror)
   "Return the contents of FILE, as a string."
-  (if (or (not file) (not (file-readable-p file)))
-      (if (not noerror)
-	  (error "Cannot read file \"%s\"" file)
-	(message "Cannot read file \"%s\"" file)
-	"")
-    (with-temp-buffer
-      (insert-file-contents file)
-      (buffer-string))))
+  (let* ((from-file (buffer-file-name (buffer-base-buffer)))
+	 (info-from-file
+	  (if from-file
+	      (concat " (referenced in file \"" from-file "\")")
+	    "")))
+    (if (or (not file) (not (file-readable-p file)))
+	(if (not noerror)
+	    (error "Cannot read file \"%s\"%s" file info-from-file)
+	  (message "Cannot read file \"%s\"%s" file info-from-file))
+      (with-temp-buffer
+	(insert-file-contents file)
+	(buffer-string)))))
 
 (defun org-extract-log-state-settings (x)
   "Extract the log state setting from a TODO keyword string.
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-02-18 14:55   ` Sebastien Vauban
@ 2014-02-18 20:21     ` Nicolas Goaziou
  2014-02-18 20:32       ` Sebastien Vauban
  0 siblings, 1 reply; 8+ messages in thread
From: Nicolas Goaziou @ 2014-02-18 20:21 UTC (permalink / raw)
  To: Sebastien Vauban; +Cc: public-emacs-orgmode-mXXj517/zsQ



Hello,

"Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> This should answer your (fruitful) comments.

Thank you for the update.

It looks good. AFAIC, you can push it.

> +    (if (or (not file) (not (file-readable-p file)))
> +	(if (not noerror)
> +	    (error "Cannot read file \"%s\"%s" file info-from-file)
> +	  (message "Cannot read file \"%s\"%s" file info-from-file))
> +      (with-temp-buffer
> +	(insert-file-contents file)
> +	(buffer-string)))))

Minor stylistic issue: I find the following a bit easier to understand.

  (if (and file (file-readable-p file))
      (with-temp-buffer
        (insert-file-contents file)
        (buffer-string))
    (funcall (if noerror #'message #'error)
             "Cannot read file \"%s\"%s" file info-from-file))


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-02-18 20:21     ` Nicolas Goaziou
@ 2014-02-18 20:32       ` Sebastien Vauban
  2014-02-18 22:10         ` Nicolas Goaziou
  0 siblings, 1 reply; 8+ messages in thread
From: Sebastien Vauban @ 2014-02-18 20:32 UTC (permalink / raw)
  To: emacs-orgmode-mXXj517/zsQ

Hello Nicolas,

Nicolas Goaziou wrote:
> "Sebastien Vauban" writes:
>
>> This should answer your (fruitful) comments.
>
> Thank you for the update.
>
> It looks good.

Thanks.

> AFAIC, you can push it.

I don't have (yet) push access...

>> +    (if (or (not file) (not (file-readable-p file)))
>> +	(if (not noerror)
>> +	    (error "Cannot read file \"%s\"%s" file info-from-file)
>> +	  (message "Cannot read file \"%s\"%s" file info-from-file))
>> +      (with-temp-buffer
>> +	(insert-file-contents file)
>> +	(buffer-string)))))
>
> Minor stylistic issue: I find the following a bit easier to understand.
>
>   (if (and file (file-readable-p file))
>       (with-temp-buffer
>         (insert-file-contents file)
>         (buffer-string))
>     (funcall (if noerror #'message #'error)
>              "Cannot read file \"%s\"%s" file info-from-file))

So do I...

Best regards,
  Seb

-- 
Sebastien Vauban

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Improve message when file to include is missing
  2014-02-18 20:32       ` Sebastien Vauban
@ 2014-02-18 22:10         ` Nicolas Goaziou
  0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Goaziou @ 2014-02-18 22:10 UTC (permalink / raw)
  To: Sebastien Vauban; +Cc: public-emacs-orgmode-mXXj517/zsQ



"Sebastien Vauban" <sva-news-D0wtAvR13HarG/iDocfnWg@public.gmane.org>
writes:

> I don't have (yet) push access...

OK. Applied, with a small refactoring.

Thank you.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-02-18 22:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 10:49 [PATCH] Improve message when file to include is missing Sebastien Vauban
2014-01-24 11:14 ` Nicolas Goaziou
2014-01-24 11:19   ` Bastien
2014-02-18 14:55   ` Sebastien Vauban
2014-02-18 20:21     ` Nicolas Goaziou
2014-02-18 20:32       ` Sebastien Vauban
2014-02-18 22:10         ` Nicolas Goaziou
2014-01-24 11:15 ` Bastien

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).