[PATCH] Improve message when file to include is missing

[PATCH] Improve message when file to include is missing

[Sebastien Vauban]

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

Re: [PATCH] Improve message when file to include is missing

[Nicolas Goaziou]

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

Re: [PATCH] Improve message when file to include is missing

[Bastien]

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

Re: [PATCH] Improve message when file to include is missing

[Sebastien Vauban]

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

Re: [PATCH] Improve message when file to include is missing

[Nicolas Goaziou]

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

Re: [PATCH] Improve message when file to include is missing

[Sebastien Vauban]

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

Re: [PATCH] Improve message when file to include is missing

[Nicolas Goaziou]

"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

Re: [PATCH] Improve message when file to include is missing

[Bastien]

"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