Den tors 31 juli 2025 kl 18:58 skrev Nathan Hartman < hartman.nat...@gmail.com>:
> On Thu, Jul 31, 2025 at 11:14 AM Daniel Sahlberg < > daniel.l.sahlb...@gmail.com> wrote: > >> Den tors 31 juli 2025 kl 15:46 skrev Nathan Hartman < >> hartman.nat...@gmail.com>: >> >>> [to -= commits@] >>> >> >>> On Tue, Jul 29, 2025 at 3:12 AM Daniel Sahlberg < >>> daniel.l.sahlb...@gmail.com> wrote: >>> >>>> Den tis 29 juli 2025 kl 09:06 skrev <dsahlb...@apache.org>: >>>> >>>>> Author: dsahlberg >>>>> Date: Tue Jul 29 07:06:11 2025 >>>>> New Revision: 1927514 >>>>> >>>>> Log: >>>>> In site/staging >>>>> >>>>> * docs/community-guide/conventions.part.html >>>>> (#crediting): search-svnlog.pl will run `svn log` itself if no >>>>> filename given >>>>> >>>>> Modified: >>>>> subversion/site/staging/docs/community-guide/conventions.part.html >>>>> >>>>> Modified: >>>>> subversion/site/staging/docs/community-guide/conventions.part.html >>>>> >>>>> ============================================================================== >>>>> --- >>>>> subversion/site/staging/docs/community-guide/conventions.part.html Tue >>>>> Jul >>>>> 29 06:45:02 2025 (r1927513) >>>>> +++ >>>>> subversion/site/staging/docs/community-guide/conventions.part.html Tue >>>>> Jul >>>>> 29 07:06:11 2025 (r1927514) >>>>> @@ -1252,7 +1252,7 @@ these fields in practice. This command >>>>> working copy will help:</p> >>>>> >>>>> <pre> >>>>> -svn log | contrib/client-side/search-svnlog.pl >>>>> "(Patch|Review|Suggested) by: " >>>>> +svn log | contrib/client-side/search-svnlog.pl -f - >>>>> "(Patch|Review|Suggested) by: " >>>>> </pre> >>>>> >>>>> <p><b>Note:</b> The "Approved by: " field seen in some >>>>> >>>>> >>>> I'm not quite sure about this change actually, I made it just to get a >>>> commit message to start discussing... >>>> >>>> The other option I considered (which is clearer in my opinion): >>>> >>>> <pre> >>>> -svn log | contrib/client-side/search-svnlog.pl "(Patch|Review|Suggested) >>>> by: " >>>> + ./contrib/client-side/search-svnlog.pl "(Patch|Review|Suggested) by: >>>> " >>>> </pre> >>>> >>>> On the other hand with the variant committed, you can check just the >>>> last 100 log messages like this: >>>> $ svn log -l100 | contrib/client-side/search-svnlog.pl -f - >>>> "(Patch|Review|Suggested) by: " >>>> >>>> Opinions? >>>> >>>> Compare the actual sites, the example is at the very end on the >>>> #crediting section: >>>> >>>> https://subversion.apache.org/docs/community-guide/conventions.html#crediting >>>> >>>> https://subversion-staging.apache.org/docs/community-guide/conventions.html#crediting >>>> >>>> Cheers, >>>> Daniel >>>> >>> >>> >>> So, without '-f -' to explicitly open stdin, search-svnlog.pl ignores >>> stdin and runs 'svn log' itself. >>> >> >> That is correct. >> >> >>> >>> This means the command shown at HACKING before the change would run >>> 'svn log' twice: first time to pipe its output to search-svnlog.pl, >>> which happily ignores it, and again from within search-svnlog.pl. >>> >> >> Also correct. >> >> Do you have a preference for one of the two possible ways to change? I >> really like that I'm able to limit the amount of messages to search with >> the "-f -" variation, but it is not so intuitive... >> > > Not a strong preference, but I prefer the change as committed because, > as you point out, you can limit the amount of messages. You could even > get fancy with things like 'svn log --revision { DATE } : HEAD'. > > If you aren't happy with the change, you could show both variations: > > Near the end of Crediting [1] > > [[[ > > Look over Subversion's existing log messages to see how to use these > fields in practice. This command from the top of your trunk working > copy will help: > > $ contrib/client-side/search-svnlog.pl "(Patch|Review|Suggested) by: " > > The script runs 'svn log' internally. You can customize the invocation > of 'svn log' by piping its output to the script; the '-f -' arguments > are required for this. For example, to search only the last 100 log > messages: > > $ svn log -l 100 | contrib/client-side/search-svnlog.pl -f - > "(Patch|Review|Suggested) by: " > > ]]] > > Or something like that... > > [1] > https://subversion-staging.apache.org/docs/community-guide/conventions.html#crediting > > Cheers, > Nathan > > Thanks, that is a very good suggestion! I've committed in r1927555 and merged to trunk. Cheers, Daniel