Bug found (but don't know to fix)

Bug found (but don't know to fix)

[Wanrong Lin]

[-- Attachment #1.1: Type: text/plain, Size: 1781 bytes --]

Hi Carsten:

I have observed that in Windows "org-open-at-point" (bound to RET key in 
my config) does not work on something like this:

file:\\hostname\path

Today I dug a little bit into it, and found the problem is here (in red):

(defun org-open-file (path &optional in-emacs line search)
  "Open the file at PATH.
First, this expands any special file name abbreviations.  Then the
configuration variable `org-file-apps' is checked if it contains an
entry for this file type, and if yes, the corresponding command is launched.
If no application is found, Emacs simply visits the file.
With optional argument IN-EMACS, Emacs will visit the file.
Optional LINE specifies a line to go to, optional SEARCH a string to
search for.  If LINE or SEARCH is given, the file will always be
opened in Emacs.
If the file does not exist, an error is thrown."
  (setq in-emacs (or in-emacs line search))
  (let* ((file (if (equal path "")
           buffer-file-name
         (substitute-in-file-name *(expand-file-name path)*)))

*(expand-file-name path) *replaces all backslashes with forward slashes, 
and later in the same function


    (if search (org-link-search search))))
     ((consp cmd)
      *(eval cmd)*)

Here "cmd" variable is "(w32-shell-execute "open" file)", and 
w32-shell-execute will complain about the file not existing.

When in-emacs is t, everything works fine, as Emacs understand both 
forward and backward slashes.

Also, it works fine on regular file path like this:
 file:c:\path\file.txt

That is because somehow my Windows system is setup (by our IT guys) to 
understand both forward and backward slashes, but that only works on 
regular file paths, not the Windows shared directory paths.

I wonder whether this can get fixed. Thank you very much.

Wanrong


[-- Attachment #1.2: Type: text/html, Size: 2522 bytes --]

[-- Attachment #2: Type: text/plain, Size: 204 bytes --]

_______________________________________________
Emacs-orgmode mailing list
Remember: use `Reply All' to send replies to the list.
Emacs-orgmode@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-orgmode

Re: Bug found (but don't know to fix)

[Carsten Dominik]

[-- Attachment #1.1: Type: text/plain, Size: 2732 bytes --]

Hi Wanrong,

how about the following patch:

- Carsten

diff --git a/lisp/org.el b/lisp/org.el
index 9094204..ee1b923 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7385,7 +7385,9 @@ If the file does not exist, an error is thrown."
  	(setq cmd (replace-match "%s" t t cmd)))
        (while (string-match "%s" cmd)
  	(setq cmd (replace-match
-		   (save-match-data (shell-quote-argument file))
+		   (save-match-data
+		     (shell-quote-argument
+		      (convert-standard-filename file)))
  		   t t cmd)))
        (save-window-excursion
  	(start-process-shell-command cmd nil cmd)






On May 19, 2008, at 11:59 PM, Wanrong Lin wrote:

> Hi Carsten:
>
> I have observed that in Windows "org-open-at-point" (bound to RET  
> key in my config) does not work on something like this:
>
> file:\\hostname\path
>
> Today I dug a little bit into it, and found the problem is here (in  
> red):
>
> (defun org-open-file (path &optional in-emacs line search)
>   "Open the file at PATH.
> First, this expands any special file name abbreviations.  Then the
> configuration variable `org-file-apps' is checked if it contains an
> entry for this file type, and if yes, the corresponding command is  
> launched.
> If no application is found, Emacs simply visits the file.
> With optional argument IN-EMACS, Emacs will visit the file.
> Optional LINE specifies a line to go to, optional SEARCH a string to
> search for.  If LINE or SEARCH is given, the file will always be
> opened in Emacs.
> If the file does not exist, an error is thrown."
>   (setq in-emacs (or in-emacs line search))
>   (let* ((file (if (equal path "")
>            buffer-file-name
>          (substitute-in-file-name (expand-file-name path))))
>
> (expand-file-name path) replaces all backslashes with forward  
> slashes, and later in the same function
>
>
>     (if search (org-link-search search))))
>      ((consp cmd)
>       (eval cmd))
>
> Here "cmd" variable is "(w32-shell-execute "open" file)", and w32- 
> shell-execute will complain about the file not existing.
>
> When in-emacs is t, everything works fine, as Emacs understand both  
> forward and backward slashes.
>
> Also, it works fine on regular file path like this:
>  file:c:\path\file.txt
>
> That is because somehow my Windows system is setup (by our IT guys)  
> to understand both forward and backward slashes, but that only works  
> on regular file paths, not the Windows shared directory paths.
>
> I wonder whether this can get fixed. Thank you very much.
>
> Wanrong
>
> _______________________________________________
> Emacs-orgmode mailing list
> Remember: use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode


[-- Attachment #1.2: Type: text/html, Size: 4502 bytes --]

[-- Attachment #2: Type: text/plain, Size: 204 bytes --]

_______________________________________________
Emacs-orgmode mailing list
Remember: use `Reply All' to send replies to the list.
Emacs-orgmode@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-orgmode

Re: Bug found (but don't know to fix)

[Wanrong Lin]

Hi, Carsten,

Thanks a lot for the patch. I tried it, but it does not work. I traced 
into the "org-open-at-point" function and found the patch is not 
executed because the condition *"(stringp cmd)" *is not satisfied, as 
"cmd" is a list "(w32-shell-execute "open" file)". This function seems a 
little bit too complicated for me to sort out, would you mind taking 
another look into it, or giving some suggestion on how to fix it? Thank you.

Wanrong


    (cond
     ((and *(stringp cmd)* (not (string-match "^\\s-*$" cmd)))
      ;; Remove quotes around the file name - we'll use 
shell-quote-argument.
      (while (string-match "['\"]%s['\"]" cmd)
        (setq cmd (replace-match "%s" t t cmd)))
      (while (string-match "%s" cmd)
        (setq cmd (replace-match
                   (save-match-data
                     (shell-quote-argument                   
                      (convert-standard-filename file)))
                   t t cmd)))
      (save-window-excursion
        (start-process-shell-command cmd nil cmd)
        (and (boundp 'org-wait) (numberp org-wait) (sit-for org-wait))
        ))


Carsten Dominik wrote:
> Hi Wanrong,
>
> how about the following patch:
>
> - Carsten
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 9094204..ee1b923 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -7385,7 +7385,9 @@ If the file does not exist, an error is thrown."
>   (setq cmd (replace-match "%s" t t cmd)))
>        (while (string-match "%s" cmd)
>   (setq cmd (replace-match
> -   (save-match-data (shell-quote-argument file))
> +   (save-match-data
> +     (shell-quote-argument
> +      (convert-standard-filename file)))
>     t t cmd)))
>        (save-window-excursion
>   (start-process-shell-command cmd nil cmd)
>
>
>
>
>
>
> On May 19, 2008, at 11:59 PM, Wanrong Lin wrote:
>> Hi Carsten:
>>
>> I have observed that in Windows "org-open-at-point" (bound to RET key 
>> in my config) does not work on something like this:
>>
>> file:\\hostname\path
>>
>> Today I dug a little bit into it, and found the problem is here (in red):
>>
>> (defun org-open-file (path &optional in-emacs line search)
>>   "Open the file at PATH.
>> First, this expands any special file name abbreviations.  Then the
>> configuration variable `org-file-apps' is checked if it contains an
>> entry for this file type, and if yes, the corresponding command is 
>> launched.
>> If no application is found, Emacs simply visits the file.
>> With optional argument IN-EMACS, Emacs will visit the file.
>> Optional LINE specifies a line to go to, optional SEARCH a string to
>> search for.  If LINE or SEARCH is given, the file will always be
>> opened in Emacs.
>> If the file does not exist, an error is thrown."
>>   (setq in-emacs (or in-emacs line search))
>>   (let* ((file (if (equal path "")
>>            buffer-file-name
>>          (substitute-in-file-name *(expand-file-name path)*)))
>>
>> *(expand-file-name path) *replaces all backslashes with forward 
>> slashes, and later in the same function
>>
>>
>>     (if search (org-link-search search))))
>>      ((consp cmd)
>>       *(eval cmd)*)
>>
>> Here "cmd" variable is "(w32-shell-execute "open" file)", and 
>> w32-shell-execute will complain about the file not existing.
>>
>> When in-emacs is t, everything works fine, as Emacs understand both 
>> forward and backward slashes.
>>
>> Also, it works fine on regular file path like this:
>>  file:c:\path\file.txt
>>
>> That is because somehow my Windows system is setup (by our IT guys) 
>> to understand both forward and backward slashes, but that only works 
>> on regular file paths, not the Windows shared directory paths.
>>
>> I wonder whether this can get fixed. Thank you very much.
>>
>> Wanrong
>>
>> _______________________________________________
>> Emacs-orgmode mailing list
>> Remember: use `Reply All' to send replies to the list.
>> Emacs-orgmode@gnu.org <mailto:Emacs-orgmode@gnu.org>
>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>

Re: Bug found (but don't know to fix)

[Carsten Dominik]

On May 21, 2008, at 4:25 PM, Wanrong Lin wrote:

> Hi, Carsten,
>
> Thanks a lot for the patch. I tried it, but it does not work. I  
> traced into the "org-open-at-point" function and found the patch is  
> not executed because the condition *"(stringp cmd)" *is not  
> satisfied, as "cmd" is a list "(w32-shell-execute "open" file)".  
> This function seems a little bit too complicated for me to sort out,  
> would you mind taking another look into it, or giving some  
> suggestion on how to fix it? Thank you.

Hi Wanrong,

How about this patch then:

diff --git a/lisp/org.el b/lisp/org.el
index 9094204..4e2aa58 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7385,7 +7385,9 @@ If the file does not exist, an error is thrown."
  	(setq cmd (replace-match "%s" t t cmd)))
        (while (string-match "%s" cmd)
  	(setq cmd (replace-match
-		   (save-match-data (shell-quote-argument file))
+		   (save-match-data
+		     (shell-quote-argument
+		      (convert-standard-filename file)))
  		   t t cmd)))
        (save-window-excursion
  	(start-process-shell-command cmd nil cmd)
@@ -7398,7 +7400,8 @@ If the file does not exist, an error is thrown."
        (if line (goto-line line)
  	(if search (org-link-search search))))
       ((consp cmd)
-      (eval cmd))
+      (let ((file (convert-standard-filename file)))
+	(eval cmd)))
       (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
      (and (org-mode-p) (eq old-mode 'org-mode)
  	 (or (not (equal old-buffer (current-buffer)))

Re: Bug found (but don't know to fix)

[Wanrong Lin]

Yes, this works! Thank you.

Wanrong

Carsten Dominik wrote:
>
> On May 21, 2008, at 4:25 PM, Wanrong Lin wrote:
>
>> Hi, Carsten,
>>
>> Thanks a lot for the patch. I tried it, but it does not work. I 
>> traced into the "org-open-at-point" function and found the patch is 
>> not executed because the condition *"(stringp cmd)" *is not 
>> satisfied, as "cmd" is a list "(w32-shell-execute "open" file)". This 
>> function seems a little bit too complicated for me to sort out, would 
>> you mind taking another look into it, or giving some suggestion on 
>> how to fix it? Thank you.
>
> Hi Wanrong,
>
> How about this patch then:
>
> diff --git a/lisp/org.el b/lisp/org.el
> index 9094204..4e2aa58 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -7385,7 +7385,9 @@ If the file does not exist, an error is thrown."
>      (setq cmd (replace-match "%s" t t cmd)))
>        (while (string-match "%s" cmd)
>      (setq cmd (replace-match
> -           (save-match-data (shell-quote-argument file))
> +           (save-match-data
> +             (shell-quote-argument
> +              (convert-standard-filename file)))
>             t t cmd)))
>        (save-window-excursion
>      (start-process-shell-command cmd nil cmd)
> @@ -7398,7 +7400,8 @@ If the file does not exist, an error is thrown."
>        (if line (goto-line line)
>      (if search (org-link-search search))))
>       ((consp cmd)
> -      (eval cmd))
> +      (let ((file (convert-standard-filename file)))
> +    (eval cmd)))
>       (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
>      (and (org-mode-p) (eq old-mode 'org-mode)
>       (or (not (equal old-buffer (current-buffer)))
>
>