On Wed, Jan 27, 2021 at 10:19 PM Yasuhito FUTATSUKI
<futat...@yf.bsdclub.org> wrote:
>
> On 2021/01/28 5:43, Johan Corveleyn wrote:
> > On Mon, Mar 16, 2020 at 5:11 AM <james...@apache.org> wrote:
> >>
> >> Author: jamessan
> >> Date: Mon Mar 16 04:11:36 2020
> >> New Revision: 1875230
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1875230&view=rev
> >> Log:
> >> Followup to r1874093, add Windows-specific argument escaping
> >>
> >> Rather than directly using apr_pescape_shell(), use apr_escape_shell() to 
> >> give
> >> an indication whether escaping is needed.  If APR reports no escaping is
> >> needed, simply surround the argument in double-quotes to handle any 
> >> embedded
> >> whitespace.
> >>
> >> When escaping is needed, on Unix we continue to use APR's escaping +
> >> post-processing for whitespace.  On Windows, perform the escaping 
> >> ourselves per
> >> these rules:
> >>
> >>   1. Surround the string with double-quotes
> >>   2. Escape any double-quotes or backslashes preceding a double-quote
> >>   3. Escape any metacharacters, including double-quotes, with ^
> >>
> >> * subversion/libsvn_subr/cmdline.c
> >>   (escape_path): Refactored as above
> >
> > I'm sorry I didn't notice this before, but on Windows, since this
> > commit r1875230 (which is included in 1.14.0) the escaping of
> > SVN_EDITOR is broken. If the envvar refers to a program with a space
> > in its path, between quotes, those quotes seem to be lost now, which
> > results in:
> >
> > [[[
> > C:\test>set EDITOR="C:\Program Files\Notepad++\notepad++" -nosession 
> > -multiInst
> >
> > C:\test>svn pe svn:ignore .
> > 'C:\Program' is not recognized as an internal or external command,
> > operable program or batch file.
> > svn: E200012: system('"C:\Program Files\Notepad++\notepad++"
> > -nosession -multiInst "svn-prop.tmp"') returned 1
> > ]]]
>
> Perhaps my pending patch also fix this.
>
> Please see
>
> https://lists.apache.org/thread.html/r2b8b0b4ca0f833e371590fadb33ee24c6335127e967664c6e612e154%40%3Cdev.subversion.apache.org%3E
>
> and attached patch
>
> https://lists.apache.org/api/email.lua?attachment=true&id=r2b8b0b4ca0f833e371590fadb33ee24c6335127e967664c6e612e154@%3Cdev.subversion.apache.org%3E&file=d76c6fedb56414d73e0d481910f7e62bde6d0582e975c4b2134c8521b5f9e30f

Yes indeed, that seems to fix it :). Nice!

Is anything holding this patch back from being committed? We're mainly
a CTR - Commit Then Review - project, so if you feel okay about it ...
I haven't studied it in depth, but on a cursory look, it seems fine to
me (and it works / fixes my problem, yay!).

-- 
Johan

Reply via email to