Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

> +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
> +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
> +	for it.  (Bug#16558)

That means we don't call Ffile_exists_p for .gz files :-(
It seems arbitrary.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 01 Feb 2014 14:26:20 -0500
> 
> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
> > +	for it.  (Bug#16558)
> 
> That means we don't call Ffile_exists_p for .gz files :-(
> It seems arbitrary.

What do you suggest? bind handlers-alist to nil?

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
>> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
>> > +	for it.  (Bug#16558)
>> That means we don't call Ffile_exists_p for .gz files :-(
>> It seems arbitrary.
> What do you suggest? bind handlers-alist to nil?

Could you describe the problem we're trying to solve?


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 01 Feb 2014 19:44:54 -0500
> 
> >> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
> >> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
> >> > +	for it.  (Bug#16558)
> >> That means we don't call Ffile_exists_p for .gz files :-(
> >> It seems arbitrary.
> > What do you suggest? bind handlers-alist to nil?
> 
> Could you describe the problem we're trying to solve?

What is unclear in its description in this bug report?

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan-W. Hahn]

Mail von Stefan Monnier, Sat, 01 Feb 2014 at 19:44:54 -0500:

Good Morning,

> >> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
> >> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
> >> > +	for it.  (Bug#16558)
> >> That means we don't call Ffile_exists_p for .gz files :-(
> >> It seems arbitrary.
> > What do you suggest? bind handlers-alist to nil?
> 
> Could you describe the problem we're trying to solve?

because I sent the bug report, I try to summarize.  Because I'm not familar
with the internals of emacs, I just use it, please throw away my thoughts if
they are nonsens.

I think the main problems are:
- to define what Fw32_shell_execute has to do before calling Shellexecute
  and what not.
- to distinct between real files and virtual objects.

There are handlers for both types defined by file-name-handler-alist
and if ShellExecute is called then windows itself also has handlers
defined be registry (HKEY_CLASSES_ROOT\CLSID\{object_clsid}\Shell\verb).

So, if calling a file-handler function inside of Fw32_shell_execute then
file-name-handler-alist transforms the calls (this was my problem with the
call to file-exists-p on an "https://url" because this was translated to a
file download, which is not intended, because I wanted just to start the
browser and in my case the url was in RESTAPI syntax too).

Afterwards windows interprets the filename (or objectname) by checking the
registry. So it knows how to handle things (i.e. virtual objects: http://url, 
outlook:00000000EFABE93C5C7E4947B0D4C936C001637FE4782400 (calls outlook), etc.).

For me, if calling w32-shell-execute, the intention is to give the control
of what to do with the object to windows.


Kind regards,
Stefan

-- 
Stefan-W. Hahn                          It is easy to make things.
                                        It is hard to make things simple.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> >> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
>> >> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
>> >> > +	for it.  (Bug#16558)
>> >> That means we don't call Ffile_exists_p for .gz files :-(
>> >> It seems arbitrary.
>> > What do you suggest? bind handlers-alist to nil?
>> Could you describe the problem we're trying to solve?
> What is unclear in its description in this bug report?

I think the problem is in calling file-exists-p.  IIUC we use it to
decide whether to pass the file to expand-file-name, right?

And the reason we do that is because some file names are "normal" and
others refer to non-files according to some w32 feature which can map
them to some other tools.

I don't know that w32 feature at all, so it's hard for me to figure out
what should be done, but it seems like file-exists-p is not the right
thing to do anyway since the file name might be "normal" but refer to
a file that doesn't exist yet.

So, how does w32 decide whether a file name is "normal" or not?


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 02 Feb 2014 10:06:37 -0500
> Cc: emacs-devel@gnu.org
> 
> >> >> > +	* w32fns.c (Fw32_shell_execute): Don't call file-exists-p for
> >> >> > +	DOCUMENT that is a "remote" file name, i.e. a file-handler exists
> >> >> > +	for it.  (Bug#16558)
> >> >> That means we don't call Ffile_exists_p for .gz files :-(
> >> >> It seems arbitrary.
> >> > What do you suggest? bind handlers-alist to nil?
> >> Could you describe the problem we're trying to solve?
> > What is unclear in its description in this bug report?
> 
> I think the problem is in calling file-exists-p.

Why is that a problem?

Btw, find-file-name-handler returns nil for file-exists-p when the
file name specifies a compressed file, so I'm not sure what bothered
you in the first place.

> IIUC we use it to decide whether to pass the file to
> expand-file-name, right?

More accurately, we use it to decide whether we need an absolute file
name.

> And the reason we do that is because some file names are "normal" and
> others refer to non-files according to some w32 feature which can map
> them to some other tools.

The reason is described in the comment: if the file name is not
absolute and its name is not relative to the directory passed to the
system API, the API will fail.  But we don't want to try to make
non-file names "absolute", since that would munge them beyond
recognition.

> I don't know that w32 feature at all, so it's hard for me to figure out
> what should be done, but it seems like file-exists-p is not the right
> thing to do anyway since the file name might be "normal" but refer to
> a file that doesn't exist yet.

Yes, that might happen, but then the file will be created in a
directory other than what the user expects it to, perhaps.

> So, how does w32 decide whether a file name is "normal" or not?

We cannot know: it's up to the application that is associated with
DOCUMENT and OPERATION.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

> Btw, find-file-name-handler returns nil for file-exists-p when the
> file name specifies a compressed file, so I'm not sure what bothered
> you in the first place.

Ha!  Indeed, I had never noticed that `operations' property!
Then I guess the current code is OK (tho it's still kind of a hack).

>> And the reason we do that is because some file names are "normal" and
>> others refer to non-files according to some w32 feature which can map
>> them to some other tools.
> The reason is described in the comment: if the file name is not
> absolute and its name is not relative to the directory passed to the
> system API, the API will fail.

I'm having trouble understanding the above: can you give an example of
a file which is neither absolute nor relative to the directory passed
to the system API?

>> I don't know that w32 feature at all, so it's hard for me to figure out
>> what should be done, but it seems like file-exists-p is not the right
>> thing to do anyway since the file name might be "normal" but refer to
>> a file that doesn't exist yet.
> Yes, that might happen, but then the file will be created in a
> directory other than what the user expects it to, perhaps.

So it would be a problem, right?

Maybe instead of Ffile_exists_p a better option is to use a w32 system
call along the lines of faccess or stat (after all, this presumed file
name will be passed to the OS rather than to Emacs functions, so it
shouldn't pay attention to file-name-handlers and things like that,
right?).
And to deal with "not yet created files" we shouldn't check the file
itself but its directory.

>> So, how does w32 decide whether a file name is "normal" or not?
> We cannot know: it's up to the application that is associated with
> DOCUMENT and OPERATION.

So we don't actually know if the string we have is really a file name,
and Ffile_exists_p is used to try and guess whether that's the case
(because we need to adjust it somehow if that's the case)?


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Btw, find-file-name-handler returns nil for file-exists-p when the
>> file name specifies a compressed file, so I'm not sure what bothered
>> you in the first place.
>
> Ha!  Indeed, I had never noticed that `operations' property!
> Then I guess the current code is OK (tho it's still kind of a hack).
>
>>> And the reason we do that is because some file names are "normal" and
>>> others refer to non-files according to some w32 feature which can map
>>> them to some other tools.
>> The reason is described in the comment: if the file name is not
>> absolute and its name is not relative to the directory passed to the
>> system API, the API will fail.
>
> I'm having trouble understanding the above: can you give an example of
> a file which is neither absolute nor relative to the directory passed
> to the system API?

No idea whether this is what is meant:

/fencepost.gnu.org:.mailrc

or variations thereof?

-- 
David Kastrup

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sun, 02 Feb 2014 21:33:18 -0500
> 
> > Btw, find-file-name-handler returns nil for file-exists-p when the
> > file name specifies a compressed file, so I'm not sure what bothered
> > you in the first place.
> 
> Ha!  Indeed, I had never noticed that `operations' property!

I'm not sure this is an accident.  IMO, any file handler that works
with local files should do the same.

> Then I guess the current code is OK (tho it's still kind of a hack).

I prefer "clever engineering solution".

> >> And the reason we do that is because some file names are "normal" and
> >> others refer to non-files according to some w32 feature which can map
> >> them to some other tools.
> > The reason is described in the comment: if the file name is not
> > absolute and its name is not relative to the directory passed to the
> > system API, the API will fail.
> 
> I'm having trouble understanding the above: can you give an example of
> a file which is neither absolute nor relative to the directory passed
> to the system API?

I think it happened to me with shr.el, since it doesn't set the
current directory of the buffer where it renders an HTML document.

> >> I don't know that w32 feature at all, so it's hard for me to figure out
> >> what should be done, but it seems like file-exists-p is not the right
> >> thing to do anyway since the file name might be "normal" but refer to
> >> a file that doesn't exist yet.
> > Yes, that might happen, but then the file will be created in a
> > directory other than what the user expects it to, perhaps.
> 
> So it would be a problem, right?

Yes; see the comments in the code.  But I don't see any good solution
to this problem; do you?

In any case, this is not worse than what happened before the change
that introduced the call to expand-file-name: then, the ShellExecute
API was always called with a file name as passed to w32-shell-execute.

> Maybe instead of Ffile_exists_p a better option is to use a w32 system
> call along the lines of faccess or stat (after all, this presumed file
> name will be passed to the OS rather than to Emacs functions, so it
> shouldn't pay attention to file-name-handlers and things like that,
> right?).

That's exactly what the current code does:

  handler = Ffind_file_name_handler (absdoc, Qfile_exists_p);
  if (NILP (handler))
    {
      Lisp_Object absdoc_encoded = ENCODE_FILE (absdoc);

      if (faccessat (AT_FDCWD, SSDATA (absdoc_encoded), F_OK, AT_EACCESS) == 0)
	document = absdoc_encoded;
      else
	document = ENCODE_FILE (document);
    }
  else
    document = ENCODE_FILE (document);

> And to deal with "not yet created files" we shouldn't check the file
> itself but its directory.

Its directory is just the default-directory of the current buffer, so
I think we are covered:

  Lisp_Object current_dir = BVAR (current_buffer, directory);;

> >> So, how does w32 decide whether a file name is "normal" or not?
> > We cannot know: it's up to the application that is associated with
> > DOCUMENT and OPERATION.
> 
> So we don't actually know if the string we have is really a file name,
> and Ffile_exists_p is used to try and guess whether that's the case
> (because we need to adjust it somehow if that's the case)?

Indeed.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Mon, 03 Feb 2014 05:12:07 +0100
> 
> > I'm having trouble understanding the above: can you give an example of
> > a file which is neither absolute nor relative to the directory passed
> > to the system API?
> 
> No idea whether this is what is meant:
> 
> /fencepost.gnu.org:.mailrc
> 
> or variations thereof?

No, I was talking about local files.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> I'm having trouble understanding the above: can you give an example of
>> a file which is neither absolute nor relative to the directory passed
>> to the system API?
> I think it happened to me with shr.el, since it doesn't set the
> current directory of the buffer where it renders an HTML document.

Sounds like a bug in the code that extracts the name from the buffer and
passes it to w32-shell-execute, or in the code that (fails to) set the
buffer's default-directory.
IOW the same would happen under GNU/Linux passing that file to call-process.

Or is there something different going on?

> In any case, this is not worse than what happened before the change
> that introduced the call to expand-file-name: then, the ShellExecute
> API was always called with a file name as passed to w32-shell-execute.

I guess the question for me now is "why call Fexpand_file_name"?
I see it was introduced in:

  timestamp: Tue 2013-12-24 19:21:06 +0200
  message:
    Minor fixes in w32-shell-execute.
  
     src/w32fns.c (Fw32_shell_execute): Ensure DOCUMENT is an absolute
     file name when it is submitted to ShellExecute.  Simplify code.
     Don't test DOCUMENT for being a string, as that is enforced by
     CHECK_STRING.  Doc fix.

But I don't know which problem it was trying to solve.

>> Maybe instead of Ffile_exists_p a better option is to use a w32 system
>> call along the lines of faccess or stat (after all, this presumed file
>> name will be passed to the OS rather than to Emacs functions, so it
>> shouldn't pay attention to file-name-handlers and things like that,
>> right?).

> That's exactly what the current code does:

>   handler = Ffind_file_name_handler (absdoc, Qfile_exists_p);
>   if (NILP (handler))
>     {
>       Lisp_Object absdoc_encoded = ENCODE_FILE (absdoc);

>       if (faccessat (AT_FDCWD, SSDATA (absdoc_encoded), F_OK, AT_EACCESS) == 0)
> 	document = absdoc_encoded;
>       else
> 	document = ENCODE_FILE (document);
>     }
>   else
>     document = ENCODE_FILE (document);

Not "exactly"; my suggestion was to use only:

      Lisp_Object absdoc_encoded = ENCODE_FILE (absdoc);
      if (faccessat (AT_FDCWD, SSDATA (absdoc_encoded), F_OK, AT_EACCESS) == 0)
	document = absdoc_encoded;
      else
	document = ENCODE_FILE (document);

Another potential tweak: we could check file-name-absolute-p before
going through the trouble of trying to figure out whether to pass the
file name to expand-file-name and/or file-exists-p (that would
also have solved the url-handler-mode issue of the OP).

>> And to deal with "not yet created files" we shouldn't check the file
>> itself but its directory.
> Its directory is just the default-directory of the current buffer,

No, the directory of "toto/titi/bar" is not default-directory.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Mon, 03 Feb 2014 09:06:33 -0500
> 
> >> I'm having trouble understanding the above: can you give an example of
> >> a file which is neither absolute nor relative to the directory passed
> >> to the system API?
> > I think it happened to me with shr.el, since it doesn't set the
> > current directory of the buffer where it renders an HTML document.
> 
> Sounds like a bug in the code that extracts the name from the buffer and
> passes it to w32-shell-execute, or in the code that (fails to) set the
> buffer's default-directory.
> IOW the same would happen under GNU/Linux passing that file to call-process.
> 
> Or is there something different going on?
> 
> > In any case, this is not worse than what happened before the change
> > that introduced the call to expand-file-name: then, the ShellExecute
> > API was always called with a file name as passed to w32-shell-execute.
> 
> I guess the question for me now is "why call Fexpand_file_name"?
> I see it was introduced in:
> 
>   timestamp: Tue 2013-12-24 19:21:06 +0200
>   message:
>     Minor fixes in w32-shell-execute.
>   
>      src/w32fns.c (Fw32_shell_execute): Ensure DOCUMENT is an absolute
>      file name when it is submitted to ShellExecute.  Simplify code.
>      Don't test DOCUMENT for being a string, as that is enforced by
>      CHECK_STRING.  Doc fix.
> 
> But I don't know which problem it was trying to solve.

The problem happened when using a function in shr.el that no longer
exists.  To recreate that, I'd need to restore that function and trace
through that.  The result I do remember: w32-shell-execute failed
because DOCUMENT was a non-absolute file name, but not in the
directory where the function expected it to be.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> I guess the question for me now is "why call Fexpand_file_name"?
[...]
>> But I don't know which problem it was trying to solve.
> The problem happened when using a function in shr.el that no longer
> exists.  To recreate that, I'd need to restore that function and trace
> through that.  The result I do remember: w32-shell-execute failed
> because DOCUMENT was a non-absolute file name, but not in the
> directory where the function expected it to be.

So maybe the simplest solution is to remove this call to
expand-file-name.  I suspect that if expand-file-name is needed it can
always be called before entering w32-shell-execute.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Mon, 03 Feb 2014 22:39:30 -0500
> 
> >> I guess the question for me now is "why call Fexpand_file_name"?
> [...]
> >> But I don't know which problem it was trying to solve.
> > The problem happened when using a function in shr.el that no longer
> > exists.  To recreate that, I'd need to restore that function and trace
> > through that.  The result I do remember: w32-shell-execute failed
> > because DOCUMENT was a non-absolute file name, but not in the
> > directory where the function expected it to be.
> 
> So maybe the simplest solution is to remove this call to
> expand-file-name.

Not sure which problem needs a solution here, but in any case,
removing the expand-file-name call will get us back the original bug,
described below.

The problem that led me to introduce this call is that this succeeds:

  (w32-shell-execute "open" "file")

but this fails:

  (w32-shell-execute "open" "dir/file")

IOW, the DOCUMENT arg must either be an absolute file name, or live
inside default-directory, not in one of its subdirectories.  It cannot
be a relative file name that includes leading directories.

> I suspect that if expand-file-name is needed it can always be called
> before entering w32-shell-execute.

That would be unusual: Emacs primitives generally expand their
file-name arguments internally, they don't require that their callers
do that.  Why should this primitive be different?

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

> The problem that led me to introduce this call is that this succeeds:
>   (w32-shell-execute "open" "file")
> but this fails:
>   (w32-shell-execute "open" "dir/file")
> IOW, the DOCUMENT arg must either be an absolute file name, or live
> inside default-directory, not in one of its subdirectories.

Ah, that makes it more clear.  We should include this example in the comments.
It does beg the question: what does

   (w32-shell-execute "open" "dir/file")

do, then?  You say it "fails", but does it signal an error?  If so, why?
My world view is very POSIX-centric, so I'm probably missing the obvious.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Tue, 04 Feb 2014 15:35:25 -0500
> 
> > The problem that led me to introduce this call is that this succeeds:
> >   (w32-shell-execute "open" "file")
> > but this fails:
> >   (w32-shell-execute "open" "dir/file")
> > IOW, the DOCUMENT arg must either be an absolute file name, or live
> > inside default-directory, not in one of its subdirectories.
> 
> Ah, that makes it more clear.  We should include this example in the comments.

Please see the new-and-improved commentary there.

> It does beg the question: what does
> 
>    (w32-shell-execute "open" "dir/file")
> 
> do, then?  You say it "fails", but does it signal an error?

Yes, it says the file was not found (a.k.a. ENOENT).

> If so, why?

Looks like a limitation of the OS API we use there.  The error code
comes directly from the API.

> My world view is very POSIX-centric, so I'm probably missing the obvious.

This API uses the "file association" infrastructure to find the
application registered to open a file.  Perhaps that infrastructure
cannot cope with relative file names that have leading directories?
That's a guess; the MS documentation doesn't say this clearly, which
is why this code was not written like that to begin with.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

> This API uses the "file association" infrastructure to find the
> application registered to open a file.  Perhaps that infrastructure
> cannot cope with relative file names that have leading directories?

Yuck!

So I guess the best we can do is:

  (if (file-name-absolute-p file) <use file>
    (let ((dir (file-name-directory file)))
      (if (null dir) <use file>
        (if (faccess dir) <use (expand-file-name file)>
          <use file>))))


-- Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Tue, 04 Feb 2014 21:38:12 -0500
> 
> So I guess the best we can do is:
> 
>   (if (file-name-absolute-p file) <use file>
>     (let ((dir (file-name-directory file)))
>       (if (null dir) <use file>
>         (if (faccess dir) <use (expand-file-name file)>
>           <use file>))))

Yes, except that the primitives you use here only work on file names,
and might go bananas when presented with something else, e.g. a URL.
So using the above does not solve the basic problem, which is: do we
have a file name on our hands or something else?  The faccess test is
the only thing that provides us with some assurance.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> (if (file-name-absolute-p file) <use file>
>> (let ((dir (file-name-directory file)))
>> (if (null dir) <use file>
>> (if (faccess dir) <use (expand-file-name file)>
>> <use file>))))
> Yes, except that the primitives you use here only work on file names,
> and might go bananas when presented with something else, e.g. a URL.

AFAIK all magic file-name handlers will do something reasonable with
the above (file-name-absolute-p file) and (file-name-directory file).

But we could also let-bind file-name-handlers-alist to nil around those
calls (or use lower level code which does something similar).  We don't
need to call the Elisp primitives since we really only care about the
system-level's notion of absolute file name and file name separator.

> So using the above does not solve the basic problem, which is: do we
> have a file name on our hands or something else?  The faccess test is
> the only thing that provides us with some assurance.

The difference I suggest is:
- we don't check file-name-handlers
- we only check (faccess dir) rather than (faccess file), and only if
  there's a "dir".
- we don't bother with any of it if the file is already absolute.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Wed, 05 Feb 2014 09:10:46 -0500
> 
> >> (if (file-name-absolute-p file) <use file>
> >> (let ((dir (file-name-directory file)))
> >> (if (null dir) <use file>
> >> (if (faccess dir) <use (expand-file-name file)>
> >> <use file>))))
> > Yes, except that the primitives you use here only work on file names,
> > and might go bananas when presented with something else, e.g. a URL.
> 
> AFAIK all magic file-name handlers will do something reasonable with
> the above (file-name-absolute-p file) and (file-name-directory file).

Not really:

  (file-name-directory "http://foo.com/wherever/index.html")
    => "http://foo.com/wherever/"

  (file-name-absolute-p "http://foo.com/wherever/index.html")
    => nil

Moreover, no one said that any particular DOCUMENT that is passed to
this function will have an Emacs file handler.

> But we could also let-bind file-name-handlers-alist to nil around those
> calls (or use lower level code which does something similar).

We do the latter.  I don't think the former is a good idea, because I
think at least expand-file-name should allow its handler to run,
e.g. for use cases like cygwin-mount.

> We don't need to call the Elisp primitives since we really only care
> about the system-level's notion of absolute file name and file name
> separator.

Right; and we don't.

> - we don't check file-name-handlers

Why is that a good idea?  If a file name has a handler, any success in
faccess etc. can only be a (rare) coincidence.

> - we only check (faccess dir) rather than (faccess file), and only if
>   there's a "dir".

What is "dir" here?

> - we don't bother with any of it if the file is already absolute.

See above: deciding whether it is absolute is not easy.  I'm afraid we
will need most of expand-file-name's code for that anyway.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[grischka]

> It does beg the question: what does
> 
>    (w32-shell-execute "open" "dir/file")
> 
> do, then?  You say it "fails", but does it signal an error?  If so, why?
> My world view is very POSIX-centric, so I'm probably missing the obvious.

Maybe that ShellExecute doesn't like forward slashes (also not
for the workdir parameter)?

--- gr

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> Date: Wed, 05 Feb 2014 18:21:56 +0100
> From: grischka <grishka@gmx.de>
> Cc: emacs-devel@gnu.org
> 
> > It does beg the question: what does
> > 
> >    (w32-shell-execute "open" "dir/file")
> > 
> > do, then?  You say it "fails", but does it signal an error?  If so, why?
> > My world view is very POSIX-centric, so I'm probably missing the obvious.
> 
> Maybe that ShellExecute doesn't like forward slashes (also not
> for the workdir parameter)?

No, that's not it (I already tried, got same results).

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>   (file-name-directory "http://foo.com/wherever/index.html")
>     => "http://foo.com/wherever/"
>   (file-name-absolute-p "http://foo.com/wherever/index.html")
>     => nil

That look reasonable (and didn't involve any remote connection or any
such problem).

And faccess("http://foo.com/wherever/") should presumably tell you that
dir doesn't exist.  So it behaves just as well as the current code.

> Moreover, no one said that any particular DOCUMENT that is passed to
> this function will have an Emacs file handler.

Indeed, I made no such assumption.

>> But we could also let-bind file-name-handlers-alist to nil around those
>> calls (or use lower level code which does something similar).
> We do the latter.  I don't think the former is a good idea, because I
> think at least expand-file-name should allow its handler to run,
> e.g. for use cases like cygwin-mount.

I can agree for expand-file-name.  I was talking only about the 
file-name-directory and file-name-absolute-p calls.

>> - we don't check file-name-handlers
> Why is that a good idea?  If a file name has a handler, any success in
> faccess etc. can only be a (rare) coincidence.

The connection between the core problem of detecting the case of
(w32-shell-execute "dir/file") and the check of file-name-handlers is
really non-obvious.

>> - we only check (faccess dir) rather than (faccess file), and only if
>> there's a "dir".
> What is "dir" here?

See my sample code: (file-name-directory file)

>> - we don't bother with any of it if the file is already absolute.
> See above: deciding whether it is absolute is not easy.

I think we only need to use w32's own notion of "absolute".  But using
file-name-absolute-p should work as well.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Wed, 05 Feb 2014 14:09:36 -0500
> 
> >   (file-name-directory "http://foo.com/wherever/index.html")
> >     => "http://foo.com/wherever/"
> >   (file-name-absolute-p "http://foo.com/wherever/index.html")
> >     => nil
> 
> That look reasonable (and didn't involve any remote connection or any
> such problem).

How is the 2nd one reasonable?  It means we will pass it through
expand-file-name, which gives the following BS:

  (expand-file-name "http://foo.com/wherever/index.html")
    => "d:/gnu/bzr/emacs/trunk/http:/foo.com/wherever/index.html"

> And faccess("http://foo.com/wherever/") should presumably tell you that
> dir doesn't exist.  So it behaves just as well as the current code.

By sheer luck, if you ask me.

> >> - we don't check file-name-handlers
> > Why is that a good idea?  If a file name has a handler, any success in
> > faccess etc. can only be a (rare) coincidence.
> 
> The connection between the core problem of detecting the case of
> (w32-shell-execute "dir/file") and the check of file-name-handlers is
> really non-obvious.

If DOCUMENT doesn't have file handlers, it is more likely to be a
local file or directory.

> >> - we only check (faccess dir) rather than (faccess file), and only if
> >> there's a "dir".
> > What is "dir" here?
> 
> See my sample code: (file-name-directory file)
> 
> >> - we don't bother with any of it if the file is already absolute.
> > See above: deciding whether it is absolute is not easy.
> 
> I think we only need to use w32's own notion of "absolute".  But using
> file-name-absolute-p should work as well.

I don't see how your proposal is simpler than what's already there, or
better, sorry.

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Stefan Monnier]

>> That look reasonable (and didn't involve any remote connection or any
>> such problem).
> How is the 2nd one reasonable?

Why shouldn't it be?  It's in a context where url-handler-mode is
deactivated, so Emacs has no reason to presume that it's a URL.

> It means we will pass it through
> expand-file-name,

Which might be the right thing to do.

>   (expand-file-name "http://foo.com/wherever/index.html")
>     => "d:/gnu/bzr/emacs/trunk/http:/foo.com/wherever/index.html"

If d:/gnu/bzr/emacs/trunk/http:/foo.com/wherever/ exists, then I'd argue
it was the right thing to do.

>> And faccess("http://foo.com/wherever/") should presumably tell you that
>> dir doesn't exist.  So it behaves just as well as the current code.
> By sheer luck, if you ask me.

The name *is* ambiguous.  The only reason such ambiguity is frequent and
non-problematic is because conflicts rarely arise.  So I don't consider
this to be luck.  It's basically by design (design of the URL name space).

>> The connection between the core problem of detecting the case of
>> (w32-shell-execute "dir/file") and the check of file-name-handlers is
>> really non-obvious.
> If DOCUMENT doesn't have file handlers, it is more likely to be a
> local file or directory.

The correlation is weak.  I don't think this heuristic is worth much if
anything in this context.

> I don't see how your proposal is simpler than what's already there, or
> better, sorry.

It's better because all the checks are *directly* related to the problem
at hand: detecting non-absolute file names which include
a directory component.

If you only care about simpler, then we can just remove the
file-name-handler check and only rely on the faccess check.
That should work just as well.


        Stefan

Re: [Emacs-diffs] trunk r116230: Fix bug #16558 with w32-shell-execute on remote file names.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Wed, 05 Feb 2014 16:53:51 -0500
> 
> >> That look reasonable (and didn't involve any remote connection or any
> >> such problem).
> > How is the 2nd one reasonable?
> 
> Why shouldn't it be?  It's in a context where url-handler-mode is
> deactivated, so Emacs has no reason to presume that it's a URL.
> 
> > It means we will pass it through
> > expand-file-name,
> 
> Which might be the right thing to do.
> 
> >   (expand-file-name "http://foo.com/wherever/index.html")
> >     => "d:/gnu/bzr/emacs/trunk/http:/foo.com/wherever/index.html"
> 
> If d:/gnu/bzr/emacs/trunk/http:/foo.com/wherever/ exists, then I'd argue
> it was the right thing to do.

We are talking about Windows, where such file names are impossible.

> >> The connection between the core problem of detecting the case of
> >> (w32-shell-execute "dir/file") and the check of file-name-handlers is
> >> really non-obvious.
> > If DOCUMENT doesn't have file handlers, it is more likely to be a
> > local file or directory.
> 
> The correlation is weak.  I don't think this heuristic is worth much if
> anything in this context.

Even a weak correlation is something.  And this was introduced in
response to a bug report that clearly had to do with a file which did
have a handler.

> > I don't see how your proposal is simpler than what's already there, or
> > better, sorry.
> 
> It's better because all the checks are *directly* related to the problem
> at hand: detecting non-absolute file names which include
> a directory component.
> 
> If you only care about simpler, then we can just remove the
> file-name-handler check and only rely on the faccess check.
> That should work just as well.

The existing code also works, and is extensively commented to describe
the problem it solves.  I would like to refrain from unnecessary
changes, certainly during the freeze, unless you insist.