You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/09/11 18:58:41 UTC
svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Author: stsp
Date: Tue Sep 11 16:58:40 2012
New Revision: 1383480
URL: http://svn.apache.org/viewvc?rev=1383480&view=rev
Log:
Allow mulitple --search options with 'svn log'. Log messages are shown
if they match any of the provided search patterns (i.e. logical OR).
This will be extended later with a new --search-and option to allow
for AND/OR combining of search terms (idea from philip).
* subversion/svn/cl.h
(svn_cl__search_pattern_t): New data type.
(svn_cl__opt_state_t): Replace search_pattern and case_insensitive_search
with an array of svn_cl__search_pattern_t objects.
* subversion/svn/log-cmd.c
(log_receiver_baton): As above, search_pattern and case_insensitive_search
become an array of search patterns.
(match_search_patterns): New helper to match multiple patterns.
(log_entry_receiver, log_entry_receiver_xml): Call the new helper.
(svn_cl__log): Track changes made to log_receiver_baton.
* subversion/svn/main.c
(svn_cl__cmd_table): Document the effect of multiple --search options in
the output of 'svn help log'.
(add_search_pattern): New helper to create an array of search patterns
from option arguments.
(sub_main): Use new helper for processing --search and --isearch options.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout: Adjust
to new output of 'svn help log'.
* subversion/tests/cmdline/log_tests.py
(log_search): Add a simple test case for multiple --search options.
Modified:
subversion/trunk/subversion/svn/cl.h
subversion/trunk/subversion/svn/log-cmd.c
subversion/trunk/subversion/svn/main.c
subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
subversion/trunk/subversion/tests/cmdline/log_tests.py
Modified: subversion/trunk/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1383480&r1=1383479&r2=1383480&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Tue Sep 11 16:58:40 2012
@@ -108,6 +108,12 @@ typedef enum svn_cl__accept_t
svn_cl__accept_t
svn_cl__accept_from_word(const char *word);
+/* --search and --isearch option values */
+typedef struct svn_cl__search_pattern_t {
+ const char *pattern; /* glob syntax */
+ svn_boolean_t case_insensitive;
+} svn_cl__search_pattern_t;
+
/*** Mergeinfo flavors. ***/
@@ -236,8 +242,7 @@ typedef struct svn_cl__opt_state_t
svn_boolean_t show_diff; /* produce diff output (maps to --diff) */
svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
svn_boolean_t include_externals; /* Recurses (in)to file & dir externals */
- const char *search_pattern; /* pattern argument for --search */
- svn_boolean_t case_insensitive_search; /* perform case-insensitive search */
+ apr_array_header_t* search_patterns; /* pattern arguments for --search */
svn_wc_conflict_resolver_func2_t conflict_func;
void *conflict_baton;
Modified: subversion/trunk/subversion/svn/log-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/log-cmd.c?rev=1383480&r1=1383479&r2=1383480&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/log-cmd.c (original)
+++ subversion/trunk/subversion/svn/log-cmd.c Tue Sep 11 16:58:40 2012
@@ -71,10 +71,9 @@ struct log_receiver_baton
/* Stack which keeps track of merge revision nesting, using svn_revnum_t's */
apr_array_header_t *merge_stack;
- /* Log message search pattern. Log entries will only be shown if the author,
- * the log message, or a changed path matches this pattern. */
- const char *search_pattern;
- svn_boolean_t case_insensitive_search;
+ /* Log message search patterns. Log entries will only be shown if the author,
+ * the log message, or a changed path matches one of these patterns. */
+ apr_array_header_t *search_patterns;
/* Pool for persistent allocations. */
apr_pool_t *pool;
@@ -203,6 +202,38 @@ match_search_pattern(const char *search_
return FALSE;
}
+/* Match all search patterns in SEARCH_PATTERNS against AUTHOR, DATE, MESSAGE,
+ * and CHANGED_PATHS. Return TRUE if any pattern matches, else FALSE.
+ * SCRACH_POOL is used for temporary allocations. */
+static svn_boolean_t
+match_search_patterns(apr_array_header_t *search_patterns,
+ const char *author,
+ const char *date,
+ const char *message,
+ apr_hash_t *changed_paths,
+ apr_pool_t *scratch_pool)
+{
+ int i;
+ svn_boolean_t match = FALSE;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ for (i = 0; i < search_patterns->nelts; i++)
+ {
+ svn_cl__search_pattern_t p;
+
+ svn_pool_clear(iterpool);
+
+ p = APR_ARRAY_IDX(search_patterns, i, svn_cl__search_pattern_t);
+ match = match_search_pattern(p.pattern, author, date,
+ message, changed_paths,
+ p.case_insensitive, iterpool);
+ if (match)
+ break;
+ }
+ svn_pool_destroy(iterpool);
+
+ return match;
+}
/* Implement `svn_log_entry_receiver_t', printing the logs in
* a human-readable and machine-parseable format.
@@ -320,10 +351,9 @@ log_entry_receiver(void *baton,
if (! lb->omit_log_message && message == NULL)
message = "";
- if (lb->search_pattern &&
- ! match_search_pattern(lb->search_pattern, author, date, message,
- log_entry->changed_paths2,
- lb->case_insensitive_search, pool))
+ if (lb->search_patterns &&
+ ! match_search_patterns(lb->search_patterns, author, date, message,
+ log_entry->changed_paths2, pool))
{
if (log_entry->has_children)
APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) = log_entry->revision;
@@ -505,10 +535,9 @@ log_entry_receiver_xml(void *baton,
}
/* Match search pattern before XML-escaping. */
- if (lb->search_pattern &&
- ! match_search_pattern(lb->search_pattern, author, date, message,
- log_entry->changed_paths2,
- lb->case_insensitive_search, pool))
+ if (lb->search_patterns &&
+ ! match_search_patterns(lb->search_patterns, author, date, message,
+ log_entry->changed_paths2, pool))
{
if (log_entry->has_children)
APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) = log_entry->revision;
@@ -740,8 +769,7 @@ svn_cl__log(apr_getopt_t *os,
: opt_state->depth;
lb.diff_extensions = opt_state->extensions;
lb.merge_stack = apr_array_make(pool, 0, sizeof(svn_revnum_t));
- lb.search_pattern = opt_state->search_pattern;
- lb.case_insensitive_search = opt_state->case_insensitive_search;
+ lb.search_patterns = opt_state->search_patterns;
lb.pool = pool;
if (opt_state->xml)
Modified: subversion/trunk/subversion/svn/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=1383480&r1=1383479&r2=1383480&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/main.c (original)
+++ subversion/trunk/subversion/svn/main.c Tue Sep 11 16:58:40 2012
@@ -690,6 +690,8 @@ const svn_opt_subcommand_desc2_t svn_cl_
" ? matches any single character\n"
" * matches a sequence of arbitrary characters\n"
" [...] matches any of the characters listed inside the brackets\n"
+ " If multiple --search options are provided, a log message is shown if\n"
+ " it matches any of the provided search patterns.\n"
" If --limit is used in combination with --search, --limit restricts the\n"
" number of log messages searched, rather than restricting the output\n"
" to a particular number of matching log messages.\n"
@@ -1579,6 +1581,23 @@ svn_cl__check_cancel(void *baton)
return SVN_NO_ERROR;
}
+/* Add a --search or --isearch argument to OPT_STATE. */
+static void
+add_search_pattern(svn_cl__opt_state_t *opt_state,
+ const char *pattern,
+ svn_boolean_t case_insensitive,
+ apr_pool_t *result_pool)
+{
+ svn_cl__search_pattern_t p;
+
+ if (opt_state->search_patterns == NULL)
+ opt_state->search_patterns = apr_array_make(
+ result_pool, 1,
+ sizeof(svn_cl__search_pattern_t));
+ p.pattern = pattern;
+ p.case_insensitive = case_insensitive;
+ APR_ARRAY_PUSH(opt_state->search_patterns, svn_cl__search_pattern_t) = p;
+}
/*** Main. ***/
@@ -2130,11 +2149,10 @@ sub_main(int argc, const char *argv[], a
opt_state.diff.properties_only = TRUE;
break;
case opt_search:
- opt_state.search_pattern = opt_arg;
+ add_search_pattern(&opt_state, opt_arg, FALSE, pool);
break;
case opt_isearch:
- opt_state.search_pattern = opt_arg;
- opt_state.case_insensitive_search = TRUE;
+ add_search_pattern(&opt_state, opt_arg, TRUE, pool);
break;
default:
/* Hmmm. Perhaps this would be a good place to squirrel away
Modified: subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout?rev=1383480&r1=1383479&r2=1383480&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (original)
+++ subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout Tue Sep 11 16:58:40 2012
@@ -37,6 +37,8 @@ usage: 1. log [PATH][@REV]
? matches any single character
* matches a sequence of arbitrary characters
[...] matches any of the characters listed inside the brackets
+ If multiple --search options are provided, a log message is shown if
+ it matches any of the provided search patterns.
If --limit is used in combination with --search, --limit restricts the
number of log messages searched, rather than restricting the output
to a particular number of matching log messages.
Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1383480&r1=1383479&r2=1383480&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/log_tests.py Tue Sep 11 16:58:40 2012
@@ -2304,6 +2304,16 @@ def log_search(sbox):
log_chain = parse_log_output(output)
check_log_chain(log_chain, [7, 6, 3])
+ # multi-pattern search
+ exit_code, output, err = svntest.actions.run_and_verify_svn(
+ None, None, [], 'log',
+ '--search', 'for revision 3',
+ '--search', 'for revision 6',
+ '--search', 'for revision 7')
+
+ log_chain = parse_log_output(output)
+ check_log_chain(log_chain, [7, 6, 3])
+
@SkipUnless(server_has_mergeinfo)
def merge_sensitive_log_with_search(sbox):
"test 'svn log -g --search'"
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Sep 21, 2012 at 6:46 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 20.09.2012 23:42, Stefan Sperling wrote:
>> On Thu, Sep 20, 2012 at 02:30:30PM -0700, Bert Huijben wrote:
>>> Fnmatch support is in apr(-util?) and has known access/performance patterns,
>>> so we might be able to move the search handling to the server in the future
>>> for performance reasons.
>>>
>>> Complete regular expression support would at least require using a new third
>>> party library and moving it to the server is not really possible because
>>> complete regular expression processing can be way to expensive.
>> What he said. I was about to write the same response :)
>>
>> I *did* search for regex stuff in APR and other existing dependencies
>> and then settled on fnmatch. httpd has a regex engine but it's not
>> part of APR for some reason. I wouldn't have a problem using that
>> in the client if it were available, but it isn't.
>
> HTTPD uses PCRE, which has a BSD license. We could use it, but I'd
> prefer to have a much better reason for doing so.
>
> (e.g., "svn grep" could be such a reason)
Another reason might be 'svn diff -F RE' support (for matching a
"function line" with RE, like the "show-c-function" option), analogous
to the GNU diff option.
See also this thread:
http://svn.haxx.se/dev/archive-2012-01/0400.shtml ("regexp matching in
svn?")
There were some other suggestions made in that thread: regex for
svn:ignore and for authz files.
Anyway ... those might all be small reasons, but maybe they start to
add up, and the lack of a regex engine is holding us back on certain
features (none of which *have* to be added in case we would have regex
support, of course -- they all have their own design problems and
implementation work).
--
Johan
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Branko Čibej <br...@wandisco.com>.
On 20.09.2012 23:42, Stefan Sperling wrote:
> On Thu, Sep 20, 2012 at 02:30:30PM -0700, Bert Huijben wrote:
>> Fnmatch support is in apr(-util?) and has known access/performance patterns,
>> so we might be able to move the search handling to the server in the future
>> for performance reasons.
>>
>> Complete regular expression support would at least require using a new third
>> party library and moving it to the server is not really possible because
>> complete regular expression processing can be way to expensive.
> What he said. I was about to write the same response :)
>
> I *did* search for regex stuff in APR and other existing dependencies
> and then settled on fnmatch. httpd has a regex engine but it's not
> part of APR for some reason. I wouldn't have a problem using that
> in the client if it were available, but it isn't.
HTTPD uses PCRE, which has a BSD license. We could use it, but I'd
prefer to have a much better reason for doing so.
(e.g., "svn grep" could be such a reason)
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by 'Daniel Shahaf' <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Thu, Sep 20, 2012 at 23:42:14 +0200:
> On Thu, Sep 20, 2012 at 02:30:30PM -0700, Bert Huijben wrote:
> > Fnmatch support is in apr(-util?) and has known access/performance patterns,
> > so we might be able to move the search handling to the server in the future
> > for performance reasons.
> >
> > Complete regular expression support would at least require using a new third
> > party library and moving it to the server is not really possible because
> > complete regular expression processing can be way to expensive.
>
> What he said. I was about to write the same response :)
>
> I *did* search for regex stuff in APR and other existing dependencies
> and then settled on fnmatch. httpd has a regex engine but it's not
> part of APR for some reason. I wouldn't have a problem using that
> in the client if it were available, but it isn't.
Ah!
I was sure APR had regex support for some reason...
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Sep 20, 2012 at 02:30:30PM -0700, Bert Huijben wrote:
> Fnmatch support is in apr(-util?) and has known access/performance patterns,
> so we might be able to move the search handling to the server in the future
> for performance reasons.
>
> Complete regular expression support would at least require using a new third
> party library and moving it to the server is not really possible because
> complete regular expression processing can be way to expensive.
What he said. I was about to write the same response :)
I *did* search for regex stuff in APR and other existing dependencies
and then settled on fnmatch. httpd has a regex engine but it's not
part of APR for some reason. I wouldn't have a problem using that
in the client if it were available, but it isn't.
RE: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout tests/cmdline/log_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: donderdag 20 september 2012 14:26
> To: dev@subversion.apache.org
> Cc: commits@subversion.apache.org
> Subject: Re: svn commit: r1383480 - in /subversion/trunk/subversion:
> svn/cl.h svn/log-cmd.c svn/main.c
> tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
> tests/cmdline/log_tests.py
>
> stsp@apache.org wrote on Tue, Sep 11, 2012 at 16:58:41 -0000:
> > Author: stsp
> > Date: Tue Sep 11 16:58:40 2012
> > New Revision: 1383480
> >
> > URL: http://svn.apache.org/viewvc?rev=1383480&view=rev
> > Log:
> > Allow mulitple --search options with 'svn log'. Log messages are shown
> > if they match any of the provided search patterns (i.e. logical OR).
>
> Right now it is fnmatch patterns --- can we have regexes too please?
Fnmatch support is in apr(-util?) and has known access/performance patterns,
so we might be able to move the search handling to the server in the future
for performance reasons.
Complete regular expression support would at least require using a new third
party library and moving it to the server is not really possible because
complete regular expression processing can be way to expensive.
Bert
>
> I'm not sure about the UI, whether it's a --search-re flag or a
> '--search re:foo' or something else.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Fri, Sep 21, 2012 at 13:34:57 +0200:
> My point is that, if we want to add regular expression support at a
> later time, we can invent a "regex" production that'll fit into the
> query syntax, instead of doubling the number of options for each command
> that supports searching.
+1
Re: Log search UI options [was: svn commit: r1383480 ...]
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 04:18:47PM +0200, Stefan Sperling wrote:
> On Fri, Sep 21, 2012 at 01:07:24PM +0100, Julian Foad wrote:
> > One idea stands out as an immediate improvement: have the basic, simple, generic search commands be case-insensitive. Instead of the present
> >
> > --search
> > --isearch (or was it --i-search? no, just playing mind games),
> > --search-and
> > --isearch-and
> >
> > reduce the current implementation to just
> >
> > --search
> > --search-and
> >
> > and make those case-insensitive. Always.
>
> That's a great suggestion, actually, and very easy to do.
> I will do this if nobody objects.
Since I doubt that anyone objects to uncluttering the interface,
I just went ahead with this in r1388530.
Re: Log search UI options [was: svn commit: r1383480 ...]
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 01:07:24PM +0100, Julian Foad wrote:
> One idea stands out as an immediate improvement: have the basic, simple, generic search commands be case-insensitive. Instead of the present
>
> --search
> --isearch (or was it --i-search? no, just playing mind games),
> --search-and
> --isearch-and
>
> reduce the current implementation to just
>
> --search
> --search-and
>
> and make those case-insensitive. Always.
That's a great suggestion, actually, and very easy to do.
I will do this if nobody objects.
Log search UI options [was: svn commit: r1383480 ...]
Posted by Julian Foad <ju...@btopenworld.com>.
I changed the subject line. And...
Branko Čibej wrote:
> On 21.09.2012 11:28, Stefan Sperling wrote:
>> It is much more convenient to have this built in, especially on
>> platforms that don't have nice post-processing tools installed
>> by default (e.g. Windows).
>
> OK.
>
>> I've been using it several times already to find commit messages
>> for things that I only vaguely remembered -- e.g. "when did stefan2
>> fix this bug where we worked around APR file flush being broken?".
>>
>> The command "svn log --search stefan2 --search-and flush" returns
>> just two log messages (r1325899 and r1240752) to pick from.
>> For me, as a human who is bad at scanning large amounts of pages
>> in a pager even with assistance from the pager's built-in search,
>> that speeds things up. It works well for that kind of thing.
>
> Cool. So we're back to UI design. :)
>
> Caveat lector: I'm not volunteering to implement what I'm about to
> propose.
>
> How about having just the --search option and creating a simple query
> language parser using parenthesis, AND, NOT and OR operators? I'd even
> consider making such searching always case-insensitive, at least for log
> messages;
One idea stands out as an immediate improvement: have the basic, simple, generic search commands be case-insensitive. Instead of the present
--search
--isearch (or was it --i-search? no, just playing mind games),
--search-and
--isearch-and
reduce the current implementation to just
--search
--search-and
and make those case-insensitive. Always.
The point is, if the aim is to provide a simple-to-use search that "just works" for typical human needs, I think just being case-insensitive will achieve this better.
[...]
> My point is that, if we want to add regular expression support at a
> later time, we can invent a "regex" production that'll fit into
> the
> query syntax, instead of doubling the number of options for each command
> that supports searching.
Yes, that argument is a good one, as long as we don't make the expression syntax too arcane.
- Julian
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 06:00:08AM -0700, C. Michael Pilato wrote:
> On 09/21/2012 04:34 AM, Branko Čibej wrote:
> > How about having just the --search option and creating a simple query
> > language parser using parenthesis, AND, NOT and OR operators? I'd even
> > consider making such searching always case-insensitive, at least for log
> > messages; alternatively, the query language could have a CI operator
> > that made the associated subquery case-insensitive (and the obverse CS
> > operator).
>
> These was my same immediate first thoughts upon seeing the suggestion of
> --search-and, too. But yeah, not planning to invest any time here myself.
And this idea was also discussed on IRC back in June:
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2012-06-28#l169
What I took away from that discussion was that going down this route
has a lot of potential to get out of hand rather quickly :)
<peterS> people will then expect magic quoting as in search engines. 'foo bar' would have an explicit AND or OR (your choice) but ' "foo bar" ' would not
<peterS> oh yeah and people will expect ( ) to work
<peterS> and then, at some point, somebody will want SQL WHERE clause syntax
<stsp> yes, and then we might as well just cache all this stuff in an sqlite db and allow people use sql in the --search argument
<peterS> svn log --search "author LIKE 'phil%' AND date BETWEEN '2012-01-05' AND '2012-01-08'"
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 09/21/2012 04:34 AM, Branko Čibej wrote:
> How about having just the --search option and creating a simple query
> language parser using parenthesis, AND, NOT and OR operators? I'd even
> consider making such searching always case-insensitive, at least for log
> messages; alternatively, the query language could have a CI operator
> that made the associated subquery case-insensitive (and the obverse CS
> operator).
These was my same immediate first thoughts upon seeing the suggestion of
--search-and, too. But yeah, not planning to invest any time here myself.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Branko Čibej <br...@wandisco.com>.
On 21.09.2012 11:28, Stefan Sperling wrote:
> On Fri, Sep 21, 2012 at 11:10:57AM +0200, Branko Čibej wrote:
>> Is doing this in the log receiver really any faster than filtering in a
>> custom callback handler, or even a script?
> It's not. And it is not about the speed of the filtering itself.
> It is much more convenient to have this built in, especially on
> platforms that don't have nice post-processing tools installed
> by default (e.g. Windows).
OK.
> I've been using it several times already to find commit messages
> for things that I only vaguely remembered -- e.g. "when did stefan2
> fix this bug where we worked around APR file flush being broken?".
>
> The command "svn log --search stefan2 --search-and flush" returns
> just two log messages (r1325899 and r1240752) to pick from.
> For me, as a human who is bad at scanning large amounts of pages
> in a pager even with assistance from the pager's built-in search,
> that speeds things up. It works well for that kind of thing.
Cool. So we're back to UI design. :)
Caveat lector: I'm not volunteering to implement what I'm about to propose.
How about having just the --search option and creating a simple query
language parser using parenthesis, AND, NOT and OR operators? I'd even
consider making such searching always case-insensitive, at least for log
messages; alternatively, the query language could have a CI operator
that made the associated subquery case-insensitive (and the obverse CS
operator).
Here's a quick stab at a BNF:
term :== atom | '"' atom... '"';
expr :== term | prefix-expr | infox-expr | paren-expres;
prefix-expr :== NOT expr | CI expr | CS expr;
infix-expr :== expr AND expr | expr OR expr;
paren-expr :== '(' expr ')';
atom :== /[^ ()]*/;
This shouldn't be any harder to write than the skel parser; and there
might already be ALv2-licensed code available that does something similar.
My point is that, if we want to add regular expression support at a
later time, we can invent a "regex" production that'll fit into the
query syntax, instead of doubling the number of options for each command
that supports searching.
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 11:10:57AM +0200, Branko Čibej wrote:
> Is doing this in the log receiver really any faster than filtering in a
> custom callback handler, or even a script?
It's not. And it is not about the speed of the filtering itself.
It is much more convenient to have this built in, especially on
platforms that don't have nice post-processing tools installed
by default (e.g. Windows).
I've been using it several times already to find commit messages
for things that I only vaguely remembered -- e.g. "when did stefan2
fix this bug where we worked around APR file flush being broken?".
The command "svn log --search stefan2 --search-and flush" returns
just two log messages (r1325899 and r1240752) to pick from.
For me, as a human who is bad at scanning large amounts of pages
in a pager even with assistance from the pager's built-in search,
that speeds things up. It works well for that kind of thing.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Branko Čibej <br...@wandisco.com>.
On 21.09.2012 11:07, Stefan Sperling wrote:
> On Fri, Sep 21, 2012 at 09:37:32AM +0100, Daniel Shahaf wrote:
>> Stefan Sperling wrote on Fri, Sep 21, 2012 at 10:07:13 +0200:
>>> Overall, I like all of your suggestions, but I don't see a need for
>>> us to implement them all for 1.8. In my mind, these are nice bite-size
>>> tasks for our issue tracker that new contributors could pick up.
>>> So I'd suggest filing a "bite-sized" issue for each of the above.
>> Disagree. The big problem is compatibility: once you have
>> --search --isearch --search-and --isearch-and
>> you have committed to that particular UI. Let's figure out a UI _now_
>> that can stand all the extensions discussed (case-insensitive,
>> logical and/or/not, regexes, etc).
> Geez, the perfect is the enemy of the good.
>
> As for regex support, well, I'm not gonna do the heavy lifting
> of integrating a regex engine into our code base for 1.8.
>
> Really, I implemented this search stuff for simple things like
> filtering log messages on stuff like 'Patch by', not to implement
> my own google for revprops.
Is doing this in the log receiver really any faster than filtering in a
custom callback handler, or even a script? Granted that filtering logs
server-side would save network bandwidth and turnarounds.
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Sperling wrote on Fri, Sep 21, 2012 at 11:07:20 +0200:
> On Fri, Sep 21, 2012 at 09:37:32AM +0100, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Fri, Sep 21, 2012 at 10:07:13 +0200:
> > > Overall, I like all of your suggestions, but I don't see a need for
> > > us to implement them all for 1.8. In my mind, these are nice bite-size
> > > tasks for our issue tracker that new contributors could pick up.
> > > So I'd suggest filing a "bite-sized" issue for each of the above.
> >
> > Disagree. The big problem is compatibility: once you have
> > --search --isearch --search-and --isearch-and
> > you have committed to that particular UI. Let's figure out a UI _now_
> > that can stand all the extensions discussed (case-insensitive,
> > logical and/or/not, regexes, etc).
>
> Geez, the perfect is the enemy of the good.
>
> As for regex support, well, I'm not gonna do the heavy lifting
> of integrating a regex engine into our code base for 1.8.
I'm not asking for this. I'm just asking to make what we already have
extensible.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 09:37:32AM +0100, Daniel Shahaf wrote:
> Stefan Sperling wrote on Fri, Sep 21, 2012 at 10:07:13 +0200:
> > Overall, I like all of your suggestions, but I don't see a need for
> > us to implement them all for 1.8. In my mind, these are nice bite-size
> > tasks for our issue tracker that new contributors could pick up.
> > So I'd suggest filing a "bite-sized" issue for each of the above.
>
> Disagree. The big problem is compatibility: once you have
> --search --isearch --search-and --isearch-and
> you have committed to that particular UI. Let's figure out a UI _now_
> that can stand all the extensions discussed (case-insensitive,
> logical and/or/not, regexes, etc).
Geez, the perfect is the enemy of the good.
As for regex support, well, I'm not gonna do the heavy lifting
of integrating a regex engine into our code base for 1.8.
Really, I implemented this search stuff for simple things like
filtering log messages on stuff like 'Patch by', not to implement
my own google for revprops.
That aside, don't you like the option names I suggested in response
to Julian's ideas? I think that UI is fine, and it's something
we can indeed add at a later stage.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Sperling wrote on Fri, Sep 21, 2012 at 10:07:13 +0200:
> Overall, I like all of your suggestions, but I don't see a need for
> us to implement them all for 1.8. In my mind, these are nice bite-size
> tasks for our issue tracker that new contributors could pick up.
> So I'd suggest filing a "bite-sized" issue for each of the above.
Disagree. The big problem is compatibility: once you have
--search --isearch --search-and --isearch-and
you have committed to that particular UI. Let's figure out a UI _now_
that can stand all the extensions discussed (case-insensitive,
logical and/or/not, regexes, etc).
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 21, 2012 at 03:43:00AM +0100, Julian Foad wrote:
> My first thought on the pattern matching was can we have a way to specify revisions where svn:log MATCHES "[Mm]erge" AND svn:author DOES NOT MATCH "julian".
>
> In other words, two things:
>
> 1. specify a match against a particular rev-prop rather than the current general "found anywhere";
Hmmm... maybe we could add:
--search-author
--search-date
--search-log
--search-paths
> 2. a "DOES NOT MATCH" option;
Yes, something like --search-not would be nice to have.
> But I didn't bother asking until I saw your email. I think this an area for endless possible extensions.
>
Indeed.
> 3. "Search in content of the diff"
That would be doable, but it would mean that we'd have to rejuggle
the logic in svn's log receiver somewhat. It would have to download
the diff before displaying the log message. Currently it's doing it
the other way around...
Overall, I like all of your suggestions, but I don't see a need for
us to implement them all for 1.8. In my mind, these are nice bite-size
tasks for our issue tracker that new contributors could pick up.
So I'd suggest filing a "bite-sized" issue for each of the above.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout tests/cmdline/log_tests.py
Posted by Julian Foad <ju...@btopenworld.com>.
--
Subversion Live 2012 - 2 Days: training, networking, demos & more! http://bit.ly/NgUaRi
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
----- Original Message -----
> From: Daniel Shahaf <d....@daniel.shahaf.name>
> To: dev@subversion.apache.org
> Cc: commits@subversion.apache.org
> Sent: Thursday, 20 September 2012, 17:25
> Subject: Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h svn/log-cmd.c svn/main.c tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout tests/cmdline/log_tests.py
>
> stsp@apache.org wrote on Tue, Sep 11, 2012 at 16:58:41 -0000:
>> Author: stsp
>> Date: Tue Sep 11 16:58:40 2012
>> New Revision: 1383480
>>
>> URL: http://svn.apache.org/viewvc?rev=1383480&view=rev
>> Log:
>> Allow mulitple --search options with 'svn log'. Log messages are
> shown
>> if they match any of the provided search patterns (i.e. logical OR).
>
> Right now it is fnmatch patterns --- can we have regexes too please?
My first thought on the pattern matching was can we have a way to specify revisions where svn:log MATCHES "[Mm]erge" AND svn:author DOES NOT MATCH "julian".
In other words, two things:
1. specify a match against a particular rev-prop rather than the current general "found anywhere";
2. a "DOES NOT MATCH" option;
But I didn't bother asking until I saw your email. I think this an area for endless possible extensions.
3. "Search in content of the diff"
is another one.
- Julian
> I'm not sure about the UI, whether it's a --search-re flag or a
> '--search re:foo' or something else.
>
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Tue, Sep 11, 2012 at 16:58:41 -0000:
> Author: stsp
> Date: Tue Sep 11 16:58:40 2012
> New Revision: 1383480
>
> URL: http://svn.apache.org/viewvc?rev=1383480&view=rev
> Log:
> Allow mulitple --search options with 'svn log'. Log messages are shown
> if they match any of the provided search patterns (i.e. logical OR).
Right now it is fnmatch patterns --- can we have regexes too please?
I'm not sure about the UI, whether it's a --search-re flag or a
'--search re:foo' or something else.
Re: svn commit: r1383480 - in /subversion/trunk/subversion: svn/cl.h
svn/log-cmd.c svn/main.c
tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
tests/cmdline/log_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Tue, Sep 11, 2012 at 16:58:41 -0000:
> Author: stsp
> Date: Tue Sep 11 16:58:40 2012
> New Revision: 1383480
>
> URL: http://svn.apache.org/viewvc?rev=1383480&view=rev
> Log:
> Allow mulitple --search options with 'svn log'. Log messages are shown
> if they match any of the provided search patterns (i.e. logical OR).
Right now it is fnmatch patterns --- can we have regexes too please?
I'm not sure about the UI, whether it's a --search-re flag or a
'--search re:foo' or something else.