convert-standard-filename on w32 does not preserve match-data?

convert-standard-filename on w32 does not preserve match-data?

[Lennart Borgman]

Should not the whole body of convert-standard-filename in w32-fns.el be 
protected by save-match-data? Does not match-string set match-data?

Re: convert-standard-filename on w32 does not preserve match-data?

[Stefan Monnier]

> Should not the whole body of convert-standard-filename in w32-fns.el be
> protected by save-match-data? Does not match-string set match-data?

No, I think instead that code that calls convert-standard-filename shouldn't
expect it to preserve the match-data.
Could you point us to the code that incorrectly expects it to preserve the
match data, or at least to a case where your suggested change fixes a bug?


        Stefan

Re: convert-standard-filename on w32 does not preserve match-data?

[Lennart Borgman]

Stefan Monnier wrote:
>> Should not the whole body of convert-standard-filename in w32-fns.el be
>> protected by save-match-data? Does not match-string set match-data?
>>     
>
> No, I think instead that code that calls convert-standard-filename shouldn't
> expect it to preserve the match-data.
> Could you point us to the code that incorrectly expects it to preserve the
> match data, or at least to a case where your suggested change fixes a bug?
>   

There is already a call to save-match-data in convert-standard-filename 
in w32-fns.el. I am just saying that it is in the wrong place.
 Useless. And I can't understand why it should not protect match-data?

Re: convert-standard-filename on w32 does not preserve match-data?

[Richard Stallman]

    No, I think instead that code that calls convert-standard-filename shouldn't
    expect it to preserve the match-data.

It seems cleaner to me for a general subroutine, that does a job not
specifically related to searching or matching, to preserve the match
data, rather than making all its callers take care.

Re: convert-standard-filename on w32 does not preserve match-data?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 21 Dec 2006 13:07:04 -0500
> Cc: lennart.borgman.073@student.lu.se, emacs-devel@gnu.org
> 
>     No, I think instead that code that calls convert-standard-filename shouldn't
>     expect it to preserve the match-data.
> 
> It seems cleaner to me for a general subroutine, that does a job not
> specifically related to searching or matching, to preserve the match
> data, rather than making all its callers take care.

Done.

Re: convert-standard-filename on w32 does not preserve match-data?

[Richard Stallman]

    > It seems cleaner to me for a general subroutine, that does a job not
    > specifically related to searching or matching, to preserve the match
    > data, rather than making all its callers take care.

    Done.

Thanks.

Re: convert-standard-filename on w32 does not preserve match-data?

[Stefan Monnier]

>     No, I think instead that code that calls convert-standard-filename
>     shouldn't expect it to preserve the match-data.

> It seems cleaner to me for a general subroutine, that does a job not
> specifically related to searching or matching, to preserve the match
> data, rather than making all its callers take care.

Until now the way things have worked apparently is:

  any function that is not a "simple builtin" does not need to save the
  match data.

Now what is a "simple builtin" is not clearly specified anywhere, but
I wouldn't expect convert-standard-filename to be one.

The main reason for limiting the number of functions that preserve the match
data is that, on the one hand save-match-data is a relatively costly
operation, and on the other, 99% of the uses of the match-data is
immediately after a regex-match, with very few operations in-between (if
any), so it's much more efficient performancewise and programmerwise to
place the save-match-data at the few places where the use of the match data
does not come immediately after the regexp-match, than to place around each
and every function that's not obviously related to searching or matching
(i.e. a majority of the functions).


        Stefan

Re: convert-standard-filename on w32 does not preserve match-data?

[Richard Stallman]

The time it takes to call save-match-data is irrelevant for
convert-standard-filename.

Re: convert-standard-filename on w32 does not preserve match-data?

[Stefan Monnier]

> The time it takes to call save-match-data is irrelevant for
> convert-standard-filename.

Inded, I'm only arguing against the principle of adding save-match-data
around any non-regexp-related function.

In thre particular case of convert-standard-filename, I think it would be
wrong to add save-match-data (unless there's a particular reason to do it,
of course), but I agree that it's not worth removing it either.


        Stefan

Re: convert-standard-filename on w32 does not preserve match-data?

[Lennart Borgman (gmail)]

Stefan Monnier wrote:
> In thre particular case of convert-standard-filename, I think it would be
> wrong to add save-match-data (unless there's a particular reason to do it,
> of course), but I agree that it's not worth removing it either.
>   

The code using convert-standard-filename probably already feels a bit 
overloaded. convert-standard-filename is an "add on" to what one 
actually tries to do and it is easily left out. Requiring the coder to 
also add save-match-data seems like an unnecessary burden in this case 
and I therefore think that convert-standard-filename should call 
save-match-data internally.

Re: convert-standard-filename on w32 does not preserve match-data?

[Stefan Monnier]

>> In thre particular case of convert-standard-filename, I think it would be
>> wrong to add save-match-data (unless there's a particular reason to do it,
>> of course), but I agree that it's not worth removing it either.

> The code using convert-standard-filename probably already feels a bit
> overloaded.

Which code are you talking about?


        Stefan

Re: convert-standard-filename on w32 does not preserve match-data?

[Lennart Borgman (gmail)]

Stefan Monnier wrote:
>>> In thre particular case of convert-standard-filename, I think it would be
>>> wrong to add save-match-data (unless there's a particular reason to do it,
>>> of course), but I agree that it's not worth removing it either.
>>>       
>
>   
>> The code using convert-standard-filename probably already feels a bit
>> overloaded.
>>     
>
> Which code are you talking about?
>   

I did not mean a specific case, but I did look around to find missing 
quoting before. Let me see if I remember...

When you use call-process you just need to do something like (at least 
in some cases):

    (call-process
        ...
       (convert-standard-filename (expand-file-name file))

but when using shell-command you should do

    (shell-command
        ...
        (shell-quote-argument (convert-standard-filename 
(expand-file-name file))

However this may depend on the exact circumstances. Of course, the only 
place were this is noticed is on w32. And as I said before the test for 
quoting is not correct at the moment since it should be buffer specific 
(or rather specific to the circumstances).

The difference above comes from the face that call-process (actually, on 
w32, in the behind the scenes) quotes the argument whereas shell-command 
does not if I remember correctly. (But please correct me if I am wrong now!)

In many cases the code just had expand-file-name.

I am not at all satisfied with this little mess. I think we should have 
some functions that does the above things and that SHOULD be used in our 
code. I e somthing like

        (defun file-arg-for-call-process(file, expand) ....)
        (defun file-arg-for-shell-command(file, expand) ....)

And I believe there currently are still bugs in this area.

Re: convert-standard-filename on w32 does not preserve match-data?

[Stefan Monnier]

>>>> In thre particular case of convert-standard-filename, I think it would be
>>>> wrong to add save-match-data (unless there's a particular reason to do it,
>>>> of course), but I agree that it's not worth removing it either.
>>> The code using convert-standard-filename probably already feels a bit
>>> overloaded.
>> Which code are you talking about?
> I did not mean a specific case, but I did look around to find missing
> quoting before. Let me see if I remember...

Nowhere in your examples was a need for save-match-data, so removing
save-match-data from convert-standard-filename wouldn't affect them.


        Stefan

Re: convert-standard-filename on w32 does not preserve match-data?

[Lennart Borgman (gmail)]

Stefan Monnier wrote:
> Nowhere in your examples was a need for save-match-data, so removing
> save-match-data from convert-standard-filename wouldn't affect them.
>   

I guess you are right. I do not know. That was not the point however.

Re: convert-standard-filename on w32 does not preserve match-data?

[Richard Stallman]

    The code using convert-standard-filename probably already feels a bit 
    overloaded. convert-standard-filename is an "add on" to what one 
    actually tries to do and it is easily left out. Requiring the coder to 
    also add save-match-data seems like an unnecessary burden in this case 
    and I therefore think that convert-standard-filename should call 
    save-match-data internally.

That is how I see it.

There will be some cases where for speed it is better not to do
save-match-data.  But in many cases the cleanliness of adding it
is more important than the speed of omitting it.