unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
@ 2016-09-07  9:52 Richard Copley
  2016-09-07 14:26 ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Copley @ 2016-09-07  9:52 UTC (permalink / raw)
  To: 24387

Hi Eli,

`convert-standard-filename' on Windows was changed in this commit:

commit 438b98e5b70c55129453870b870a139c4a4a77fd
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Mon Apr 25 11:50:37 2016 +0300

    Don't mirror slashes in convert-standard-filename on MS-Windows

    * lisp/w32-fns.el (w32-convert-standard-filename): Don't mirror
    slashes into backslashes.  This avoids producing ugly file names,
    and is deemed no longer necessary, and should certainly be
    unrelated to which shell is in use.

I for one /did/ use filenames created by this function in shell
commands. It was explicitly documented that it would work,
and it still is (see the docstring for `convert-standard-filename').
Now I need to take time out to roll my own version and review
all my callers (and, because I'm a nice person and like to help,
file a bug report and expose myself to the ensuing derision :) ).

Please will you change it back? If not, when that docstring gets
fixed, can it document what people should use instead?





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07  9:52 bug#24387: 25.1.50; w32-convert-standard-filenames no longer works Richard Copley
@ 2016-09-07 14:26 ` Eli Zaretskii
  2016-09-07 15:59   ` Richard Copley
  2016-09-07 16:19   ` Noam Postavsky
  0 siblings, 2 replies; 7+ messages in thread
From: Eli Zaretskii @ 2016-09-07 14:26 UTC (permalink / raw)
  To: Richard Copley; +Cc: 24387

retitle 24387 w32-convert-standard-filenames no longer mirrors slashes
thanks

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 7 Sep 2016 10:52:32 +0100
> 
> commit 438b98e5b70c55129453870b870a139c4a4a77fd
> Author: Eli Zaretskii <address@hidden>
> Date:   Mon Apr 25 11:50:37 2016 +0300
> 
>     Don't mirror slashes in convert-standard-filename on MS-Windows
> 
>     * lisp/w32-fns.el (w32-convert-standard-filename): Don't mirror
>     slashes into backslashes.  This avoids producing ugly file names,
>     and is deemed no longer necessary, and should certainly be
>     unrelated to which shell is in use.
> 
> I for one /did/ use filenames created by this function in shell
> commands.

Why did you do that?  It's a mistake: that function's purpose is
entirely different, as the ELisp manual documents:

  24.8.7 Standard File Names
  --------------------------

  Sometimes, an Emacs Lisp program needs to specify a standard file name
  for a particular use—typically, to hold configuration data specified by
  the current user.
  [...]
     A lower-level function for standardizing file names, which
  ‘locate-user-emacs-file’ uses as a subroutine, is
  ‘convert-standard-filename’.

   -- Function: convert-standard-filename filename
       This function returns a file name based on FILENAME, which fits the
       conventions of the current operating system.
  [...]
       The recommended way to use this function is to specify a name which
       fits the conventions of GNU and Unix systems, and pass it to
       ‘convert-standard-filename’.

IOW, this function is for Lisp programs that need to specify a fixed
standardized file name.  It has no relation whatsoever to shell
commands.

> It was explicitly documented that it would work,
> and it still is (see the docstring for `convert-standard-filename').

Oops!  Will fix, thanks.

> Now I need to take time out to roll my own version and review
> all my callers

Do you really need to mirror slashes in shell commands?  Most Windows
applications understand forward slashes just fine.  What problems do
you have with forward slashes in shell commands on MS-Windows?

> Please will you change it back?

That would bring back the ugliness I fixed by the commit in question.
And the use case for which you want the feature is simply wrong, you
are not supposed to use convert-standard-filename for such purposes.

> If not, when that docstring gets fixed, can it document what people
> should use instead?

I'm not sure what to say in the doc string.  On the one hand, the
function is not to be used in preparation of shell commands, so
talking about shell commands there would be wrong, IMO.  OTOH,
replacing one character with another is a trivial operation in Emacs,
something like

	(setq start 0)
	(while (string-match "/" name start)
	  (aset name (match-beginning 0) ?\\)
	  (setq start (match-end 0))))

should be all you need.

I could add the above snippet to the NEWS entry that announces the
change; would that be good enough?





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07 14:26 ` Eli Zaretskii
@ 2016-09-07 15:59   ` Richard Copley
  2016-09-07 16:30     ` Eli Zaretskii
  2016-09-07 16:19   ` Noam Postavsky
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Copley @ 2016-09-07 15:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24387

On 7 September 2016 at 15:26, Eli Zaretskii <eliz@gnu.org> wrote:
> retitle 24387 w32-convert-standard-filenames no longer mirrors slashes
> thanks
>
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Wed, 7 Sep 2016 10:52:32 +0100
>>
>> commit 438b98e5b70c55129453870b870a139c4a4a77fd
>> Author: Eli Zaretskii <address@hidden>
>> Date:   Mon Apr 25 11:50:37 2016 +0300
>>
>>     Don't mirror slashes in convert-standard-filename on MS-Windows
>>
>>     * lisp/w32-fns.el (w32-convert-standard-filename): Don't mirror
>>     slashes into backslashes.  This avoids producing ugly file names,
>>     and is deemed no longer necessary, and should certainly be
>>     unrelated to which shell is in use.
>>
>> I for one /did/ use filenames created by this function in shell
>> commands.
>
> Why did you do that?  It's a mistake: that function's purpose is
> entirely different, as the ELisp manual documents:
[...]
> IOW, this function is for Lisp programs that need to specify a fixed
> standardized file name.  It has no relation whatsoever to shell
> commands.

I see, thanks.

>> It was explicitly documented that it would work,
>> and it still is (see the docstring for `convert-standard-filename').
>
> Oops!  Will fix, thanks.
>
>> Now I need to take time out to roll my own version and review
>> all my callers
>
> Do you really need to mirror slashes in shell commands?

I do. I only investigated this because stuff stopped working.

> Most Windows
> applications understand forward slashes just fine.

I know; this is for the shell itself, so it's generally only for the names of
and/or paths to program files.

> What problems do
> you have with forward slashes in shell commands on MS-Windows?

I didn't go into detail because it's not all that interesting.
In fact I sometimes modify `exec-path' at runtime (in a fairly controlled
manner, but it's not something I'd inflict on other users); for the sake
of subprocesses, I also set the PATH environment variable in Emacs
correspondingly, using `convert-standard-filename' and `path-separator'
(the intention was to be cross-platform, as I have the same site-lisp
and init.el checked out in various places). This of course has drawbacks;
those are not your problem!

The symptom was that vc-pull stopped working if Windows git needed to
invoke MSYS ssh during `vc-pull' (git was "unable to spawn ssh"),
so there was another layer of translation involved too, which might also
have been where things went wrong.

>> Please will you change it back?
>
> That would bring back the ugliness I fixed by the commit in question.
> And the use case for which you want the feature is simply wrong, you
> are not supposed to use convert-standard-filename for such purposes.

Fair enough.

>> If not, when that docstring gets fixed, can it document what people
>> should use instead?
>
> I'm not sure what to say in the doc string.  On the one hand, the
> function is not to be used in preparation of shell commands, so
> talking about shell commands there would be wrong, IMO.  OTOH,
> replacing one character with another is a trivial operation in Emacs,
> something like
>
>         (setq start 0)
>         (while (string-match "/" name start)
>           (aset name (match-beginning 0) ?\\)
>           (setq start (match-end 0))))
>
> should be all you need.
>
> I could add the above snippet to the NEWS entry that announces the
> change; would that be good enough?

Up to you; if you want to help out other people who made the same
mistake, I expect they'd appreciate it.

The function `cygwin-convert-file-name-to-windows' exists.
Maybe you'd consider including that function(ality) in non-cygwin
builds, with an alias ((w32-)convert-file-name-to-windows?). It could
be documented in the same footnote; I think that's where it would be
easiest to discover.

Either way I accept your explanation and I will fix my code.

Thanks,
Richard.





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07 14:26 ` Eli Zaretskii
  2016-09-07 15:59   ` Richard Copley
@ 2016-09-07 16:19   ` Noam Postavsky
  2016-09-07 17:13     ` Eli Zaretskii
  1 sibling, 1 reply; 7+ messages in thread
From: Noam Postavsky @ 2016-09-07 16:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Richard Copley, 24387

On Wed, Sep 7, 2016 at 10:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Do you really need to mirror slashes in shell commands?  Most Windows
> applications understand forward slashes just fine.  What problems do
> you have with forward slashes in shell commands on MS-Windows?

Not sure if this matters, but most cmd.exe internal commands recognize
forward slashes as indicating options, and some get confused by
forward slashes in file names:

C:\Users\npostavs>dir /b C:\temp
deleteme

C:\Users\npostavs>dir /b C:/temp
Parameter format not correct - "emp".

C:\Users\npostavs>del C:/temp/deleteme
Invalid switch - "temp".
C:\Users\npostavs>del c:\temp\deleteme





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07 15:59   ` Richard Copley
@ 2016-09-07 16:30     ` Eli Zaretskii
  2016-09-07 18:59       ` Richard Copley
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2016-09-07 16:30 UTC (permalink / raw)
  To: Richard Copley; +Cc: 24387

> From: Richard Copley <rcopley@gmail.com>
> Date: Wed, 7 Sep 2016 16:59:56 +0100
> Cc: 24387@debbugs.gnu.org
> 
> >> It was explicitly documented that it would work,
> >> and it still is (see the docstring for `convert-standard-filename').
> >
> > Oops!  Will fix, thanks.

Done.

> In fact I sometimes modify `exec-path' at runtime (in a fairly controlled
> manner, but it's not something I'd inflict on other users); for the sake
> of subprocesses, I also set the PATH environment variable in Emacs
> correspondingly, using `convert-standard-filename' and `path-separator'

Not sure why you'd need to use convert-standard-filename for PATH:
that variable holds the original Windows directory names with
backslashes.  If you add to it directories you added to exec-path,
then you could simply add directories with backslashes to exec-path,
which then won't need to be mirrored.

> > I'm not sure what to say in the doc string.  On the one hand, the
> > function is not to be used in preparation of shell commands, so
> > talking about shell commands there would be wrong, IMO.  OTOH,
> > replacing one character with another is a trivial operation in Emacs,
> > something like
> >
> >         (setq start 0)
> >         (while (string-match "/" name start)
> >           (aset name (match-beginning 0) ?\\)
> >           (setq start (match-end 0))))
> >
> > should be all you need.
> >
> > I could add the above snippet to the NEWS entry that announces the
> > change; would that be good enough?
> 
> Up to you; if you want to help out other people who made the same
> mistake, I expect they'd appreciate it.

I added that to NEWS.

> The function `cygwin-convert-file-name-to-windows' exists.
> Maybe you'd consider including that function(ality) in non-cygwin
> builds, with an alias ((w32-)convert-file-name-to-windows?). It could
> be documented in the same footnote; I think that's where it would be
> easiest to discover.

cygwin-convert-file-name-to-windows relies on an internal Cygwin
function that doesn't exist on native Windows, so it must be
Cygwin-specific.  In addition, it goes through 2 encoding conversions,
something that IMO is overkill for a simple job of mirroring slashes.
(For Cygwin, I guess this is justified because of the /cygdrive/
magic, but we don't need that here.)

Thanks.  Unless you (or someone else) have further comments, I will
mark this bug done.





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07 16:19   ` Noam Postavsky
@ 2016-09-07 17:13     ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2016-09-07 17:13 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: rcopley, 24387

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Wed, 7 Sep 2016 12:19:29 -0400
> Cc: Richard Copley <rcopley@gmail.com>, 24387@debbugs.gnu.org
> 
> On Wed, Sep 7, 2016 at 10:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Do you really need to mirror slashes in shell commands?  Most Windows
> > applications understand forward slashes just fine.  What problems do
> > you have with forward slashes in shell commands on MS-Windows?
> 
> Not sure if this matters, but most cmd.exe internal commands recognize
> forward slashes as indicating options, and some get confused by
> forward slashes in file names:

I didn't mean cmd.exe when I said "most".

> C:\Users\npostavs>dir /b C:\temp
> deleteme
> 
> C:\Users\npostavs>dir /b C:/temp
> Parameter format not correct - "emp".
> 
> C:\Users\npostavs>del C:/temp/deleteme
> Invalid switch - "temp".
> C:\Users\npostavs>del c:\temp\deleteme

(Some of this will work if you quote the file names.)





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

* bug#24387: 25.1.50; w32-convert-standard-filenames no longer works
  2016-09-07 16:30     ` Eli Zaretskii
@ 2016-09-07 18:59       ` Richard Copley
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Copley @ 2016-09-07 18:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24387

On 7 September 2016 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
> Not sure why you'd need to use convert-standard-filename for PATH:
> that variable holds the original Windows directory names with
> backslashes.  If you add to it directories you added to exec-path,
> then you could simply add directories with backslashes to exec-path,
> which then won't need to be mirrored.

Depends how the added directories are specified in the first place.
Thanks, but don't bother, it's my problem and not worth your time.

> Thanks.  Unless you (or someone else) have further comments, I will
> mark this bug done.

Fine with me.





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

end of thread, other threads:[~2016-09-07 18:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-07  9:52 bug#24387: 25.1.50; w32-convert-standard-filenames no longer works Richard Copley
2016-09-07 14:26 ` Eli Zaretskii
2016-09-07 15:59   ` Richard Copley
2016-09-07 16:30     ` Eli Zaretskii
2016-09-07 18:59       ` Richard Copley
2016-09-07 16:19   ` Noam Postavsky
2016-09-07 17:13     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).