'svn list --search' feature complete? (was: re: conflict resolver status update (roll 1.10.0 alpha 1?)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

'svn list --search' feature complete? (was: re: conflict resolver status update (roll 1.10.0 alpha 1?)

Johan Corveleyn-3
On Sun, Jan 29, 2017 at 10:35 AM, Stefan Fuhrmann <[hidden email]> wrote:
> On 24.01.2017 12:20, Stefan Sperling wrote:
...

>> I would like to get an 1.10.0 alpha1 released in February. Unless I hear
>> objections I will start rolling this alpha release from trunk and call a
>> vote on it soon.
>
>
> Originally, I wanted to complete the get_list API before the alpha
> release but I can't make any commitments.  So, it is probably better
> to roll the alpha with whatever is on /trunk at that time.
>
> -- Stefan^2.

Hi Stefan2,

Is the current state of the new get_list API complete on trunk now? Is
the 'svn list --search' feature useable / good enough for beta?

Context: on IRC we were talking about releasing a 1.10 beta, so this
crossed my mind as one of the new features that might not be complete
yet.

BTW: 'svn list --search' isn't described in the release notes yet (and
neither in CHANGES I believe).

--
Johan
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete? (was: re: conflict resolver status update (roll 1.10.0 alpha 1?)

Julian Foad-5
Johan Corveleyn wrote:

> On Sun, Jan 29, 2017 at 10:35 AM, Stefan Fuhrmann <[hidden email]> wrote:
>> Originally, I wanted to complete the get_list API before the alpha
>> release but I can't make any commitments.  So, it is probably better
>> to roll the alpha with whatever is on /trunk at that time.
>
> Is the current state of the new get_list API complete on trunk now? Is
> the 'svn list --search' feature useable / good enough for beta?
> [...]
> BTW: 'svn list --search' isn't described in the release notes yet (and
> neither in CHANGES I believe).

Quickly trying it out, I notice a bug. Matching is, presumably, I hope,
intended to be case-insensitive, consistent with 'svn log --search'. It
doesn't work properly: it seems not to match filenames containing
upper-case characters:

$ svn list | grep -i "^co"
COMMITTERS
configure.ac
contrib/

$ svn list --search 'co*'
configure.ac
contrib/

$ svn list --search 'CO*'
configure.ac
contrib/


- Julian
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Julian Foad-5
Julian Foad wrote:

> Quickly trying it out, I notice a bug. Matching is, presumably, I hope,
> intended to be case-insensitive, consistent with 'svn log --search'. It
> doesn't work properly: it seems not to match filenames containing
> upper-case characters:
>
> $ svn list | grep -i "^co"
> COMMITTERS
> configure.ac
> contrib/
>
> $ svn list --search 'co*'
> configure.ac
> contrib/
>
> $ svn list --search 'CO*'
> configure.ac
> contrib/

Also the search term seems to be required to match the entire path, so I
need to write "co*" rather than just "co" to match "configure.ac". That
is inconsistent with "svn log --search" which looks for the pattern as a
substring even when matching paths. I think differences such as this are
bad.

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Daniel Shahaf-2
Julian Foad wrote on Mon, 21 Aug 2017 14:09 +0100:
> Also the search term seems to be required to match the entire path, so I
> need to write "co*" rather than just "co" to match "configure.ac". That
> is inconsistent with "svn log --search" which looks for the pattern as a
> substring even when matching paths. I think differences such as this are
> bad.

From the peanut gallery, I'm not sure about this.  When one greps log
messages, I think it is far more common to search for a _substring_ of
the log message than to ask for all revisions whose log messages are
strcmp()-equal to a particular string.

On the other hand, with paths, it's plausible that one would know the
exact basename being sought, and wouldn't be interested in filenames
that contain that basename as a substring.

Further, with glob patterns, if the CLI implements exact matching, users
can achieve either exact matching (default) or substring matching (by
adding * before and after the pattern), whereas if the client implemented
substring matching, there would be no way for users to request exact
matching: fnmatch() doesn't have an equivalent of regexes' ^ and $
anchors.

But I haven't thought much about this.  There may well be logic to the
current behaviour that I'm overlooking.

Cheers,

Daniel
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Johan Corveleyn-3
On Mon, Aug 21, 2017 at 8:14 PM, Daniel Shahaf <[hidden email]> wrote:

> Julian Foad wrote on Mon, 21 Aug 2017 14:09 +0100:
>> Also the search term seems to be required to match the entire path, so I
>> need to write "co*" rather than just "co" to match "configure.ac". That
>> is inconsistent with "svn log --search" which looks for the pattern as a
>> substring even when matching paths. I think differences such as this are
>> bad.
>
> From the peanut gallery, I'm not sure about this.  When one greps log
> messages, I think it is far more common to search for a _substring_ of
> the log message than to ask for all revisions whose log messages are
> strcmp()-equal to a particular string.
>
> On the other hand, with paths, it's plausible that one would know the
> exact basename being sought, and wouldn't be interested in filenames
> that contain that basename as a substring.
>
> Further, with glob patterns, if the CLI implements exact matching, users
> can achieve either exact matching (default) or substring matching (by
> adding * before and after the pattern), whereas if the client implemented
> substring matching, there would be no way for users to request exact
> matching: fnmatch() doesn't have an equivalent of regexes' ^ and $
> anchors.
>
> But I haven't thought much about this.  There may well be logic to the
> current behaviour that I'm overlooking.

FWIW, I agree with Daniel's argumentation here about doing exact
matching with --list (and requiring *'s for substring matching).

--
Johan
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Julian Foad-5
Johan Corveleyn wrote:

> On Mon, Aug 21, 2017 at 8:14 PM, Daniel Shahaf <[hidden email]> wrote:
>> Julian Foad wrote on Mon, 21 Aug 2017 14:09 +0100:
>>> Also the search term seems to be required to match the entire path, so I
>>> need to write "co*" rather than just "co" to match "configure.ac". That
>>> is inconsistent with "svn log --search" which looks for the pattern as a
>>> substring even when matching paths. I think differences such as this are
>>> bad.
>>
>> From the peanut gallery, I'm not sure about this.  When one greps log
>> messages, I think it is far more common to search for a _substring_ of
>> the log message than to ask for all revisions whose log messages are
>> strcmp()-equal to a particular string.
>>
>> On the other hand, with paths, it's plausible that one would know the
>> exact basename being sought, and wouldn't be interested in filenames
>> that contain that basename as a substring.
>>
>> Further, with glob patterns, if the CLI implements exact matching, users
>> can achieve either exact matching (default) or substring matching (by
>> adding * before and after the pattern), whereas if the client implemented
>> substring matching, there would be no way for users to request exact
>> matching: fnmatch() doesn't have an equivalent of regexes' ^ and $
>> anchors.
>>
>> But I haven't thought much about this.  There may well be logic to the
>> current behaviour that I'm overlooking.
>
> FWIW, I agree with Daniel's argumentation here about doing exact
> matching with --list (and requiring *'s for substring matching).

So do I.

I also think, for consistency and for the reasons above, we should
change 'svn log --search' so that when searching in the changed paths it
works this way too. Then 'foo' and '*foo*' would have different meanings
when searching in the changed paths. When searching in fields such as
the log message, it would keep the existing behaviour whereby both of
those search terms would mean a substring search.

That would be a backwards-incompatible change, but to a non-prominent
and undocumented detail of the UI, and for good reasons. Does that make
sense?

- Julian
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Stefan Fuhrmann
In reply to this post by Johan Corveleyn-3
On 18.08.2017 13:36, Johan Corveleyn wrote:

> On Sun, Jan 29, 2017 at 10:35 AM, Stefan Fuhrmann <[hidden email]> wrote:
>> On 24.01.2017 12:20, Stefan Sperling wrote:
> ...
>>> I would like to get an 1.10.0 alpha1 released in February. Unless I hear
>>> objections I will start rolling this alpha release from trunk and call a
>>> vote on it soon.
>>
>>
>> Originally, I wanted to complete the get_list API before the alpha
>> release but I can't make any commitments.  So, it is probably better
>> to roll the alpha with whatever is on /trunk at that time.
>>
>> -- Stefan^2.
>
> Hi Stefan2,
>
> Is the current state of the new get_list API complete on trunk now? Is
> the 'svn list --search' feature useable / good enough for beta?
>
> Context: on IRC we were talking about releasing a 1.10 beta, so this
> crossed my mind as one of the new features that might not be complete
> yet.
>
> BTW: 'svn list --search' isn't described in the release notes yet (and
> neither in CHANGES I believe).
>

Sorry that had fallen off the edge of the disk for
the last couple of weeks.  Been super busy and then
went fully off the grid for the last two weeks.

r1806548 fixes the inconsistency that Julian found.
r1806550 got the CHANGES updated.

My plan is to complete the ra_serf implementation
this week.  Let's hope no higher-prio SVN issue
interrupts that once more.

-- Stefan^2.
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Johan Corveleyn-3
On Tue, Aug 29, 2017 at 12:08 PM, Stefan Fuhrmann <[hidden email]> wrote:

> On 18.08.2017 13:36, Johan Corveleyn wrote:
>>
>> On Sun, Jan 29, 2017 at 10:35 AM, Stefan Fuhrmann <[hidden email]>
>> wrote:
>>>
>>> On 24.01.2017 12:20, Stefan Sperling wrote:
>>
>> ...
>>>>
>>>> I would like to get an 1.10.0 alpha1 released in February. Unless I hear
>>>> objections I will start rolling this alpha release from trunk and call a
>>>> vote on it soon.
>>>
>>>
>>>
>>> Originally, I wanted to complete the get_list API before the alpha
>>> release but I can't make any commitments.  So, it is probably better
>>> to roll the alpha with whatever is on /trunk at that time.
>>>
>>> -- Stefan^2.
>>
>>
>> Hi Stefan2,
>>
>> Is the current state of the new get_list API complete on trunk now? Is
>> the 'svn list --search' feature useable / good enough for beta?
>>
>> Context: on IRC we were talking about releasing a 1.10 beta, so this
>> crossed my mind as one of the new features that might not be complete
>> yet.
>>
>> BTW: 'svn list --search' isn't described in the release notes yet (and
>> neither in CHANGES I believe).
>>
>
> Sorry that had fallen off the edge of the disk for
> the last couple of weeks.  Been super busy and then
> went fully off the grid for the last two weeks.
>
> r1806548 fixes the inconsistency that Julian found.

Are you sure? As I understood from Julian's mail, the problem with the
case-sensitivity really seemed like a bug: files with uppercase
characters were simply not found, not even if the pattern was spelled
out in uppercase. See the last one here below:

> $ svn list | grep -i "^co"
> COMMITTERS
> configure.ac
> contrib/
>
> $ svn list --search 'co*'
> configure.ac
> contrib/
>
> $ svn list --search 'CO*'
> configure.ac
> contrib/

So that looks like: you tried to make it case-insensitive (by
converting the pattern to lowercase internally), but it doesn't work
(because the paths are not converted similarly or something).

(I haven't tested it myself)

Orthogonally: I think case-insensitive matching would be nice to have
as well (either always, or optionally with some flag / pattern prefix
/ ...). Use case: Windows user base, trying to search for files with
an extension, like *.PNG, *.png, ...

> r1806550 got the CHANGES updated.
>
> My plan is to complete the ra_serf implementation
> this week.  Let's hope no higher-prio SVN issue
> interrupts that once more.

Great! Thanks for making this work :-).

--
Johan
Reply | Threaded
Open this post in threaded view
|

Re: 'svn list --search' feature complete?

Stefan Fuhrmann
On 29.08.2017 12:22, Johan Corveleyn wrote:

> On Tue, Aug 29, 2017 at 12:08 PM, Stefan Fuhrmann <[hidden email]> wrote:
>> On 18.08.2017 13:36, Johan Corveleyn wrote:
>>>
>>> On Sun, Jan 29, 2017 at 10:35 AM, Stefan Fuhrmann <[hidden email]>
>>> wrote:
>>>>
>>>> On 24.01.2017 12:20, Stefan Sperling wrote:
>>>
>>> ...
>>>>>
>>>>> I would like to get an 1.10.0 alpha1 released in February. Unless I hear
>>>>> objections I will start rolling this alpha release from trunk and call a
>>>>> vote on it soon.
>>>>
>>>>
>>>>
>>>> Originally, I wanted to complete the get_list API before the alpha
>>>> release but I can't make any commitments.  So, it is probably better
>>>> to roll the alpha with whatever is on /trunk at that time.
>>>>
>>>> -- Stefan^2.
>>>
>>>
>>> Hi Stefan2,
>>>
>>> Is the current state of the new get_list API complete on trunk now? Is
>>> the 'svn list --search' feature useable / good enough for beta?
>>>
>>> Context: on IRC we were talking about releasing a 1.10 beta, so this
>>> crossed my mind as one of the new features that might not be complete
>>> yet.
>>>
>>> BTW: 'svn list --search' isn't described in the release notes yet (and
>>> neither in CHANGES I believe).
>>>
>>
>> Sorry that had fallen off the edge of the disk for
>> the last couple of weeks.  Been super busy and then
>> went fully off the grid for the last two weeks.
>>
>> r1806548 fixes the inconsistency that Julian found.
>
> Are you sure? As I understood from Julian's mail, the problem with the
> case-sensitivity really seemed like a bug: files with uppercase
> characters were simply not found, not even if the pattern was spelled
> out in uppercase. See the last one here below:
>
>> $ svn list | grep -i "^co"
>> COMMITTERS
>> configure.ac
>> contrib/
>>
>> $ svn list --search 'co*'
>> configure.ac
>> contrib/
>>
>> $ svn list --search 'CO*'
>> configure.ac
>> contrib/
>
> So that looks like: you tried to make it case-insensitive (by
> converting the pattern to lowercase internally), but it doesn't work
> (because the paths are not converted similarly or something).
>
> (I haven't tested it myself)

The thing is that --search "normalizes" the given patterns
to lower-case and handles accents. --patterns only converts
to UTF-8.

> Orthogonally: I think case-insensitive matching would be nice to have
> as well (either always, or optionally with some flag / pattern prefix
> / ...). Use case: Windows user base, trying to search for files with
> an extension, like *.PNG, *.png, ...

Not sure that will work on the server-side.

-- Stefan^2.