[darcs-users] [patch222] Add support for matching against log messages [status=amend-requested]

Eric Kow kowey at darcs.net
Tue Apr 27 15:28:59 UTC 2010


Hi Matthias,

 [Trent, there's a small note for you below]

Looks good (mostly).  I was just about to apply this when I noticed a
couple of very minor issues.  These are trivial enough for me to ignore,
but since there's a couple of them, I figured I may as well ask for an
amendment.

Amending patches
----------------
Handy tip for the darcs.net patch tracker: you can submit amendments to
patches by just saying

   darcs send --subject '[patch222]'

Doing so saves you the trouble of keeping track of multiple patch objects and
also makes the review history easier to follow.  For more patch tracker hints,
see http://wiki.darcs.net/Development/PatchTracker.

Requests
--------
1. Please remove the tabs in the new import lines.
   Perhaps you could persuade your text editor to switch to spaces-only
   mode for Haskell code?

2. Patch name could probably be 'resolve issue1769: etc'
   (but check to make sure it's really the same thing)

3. Example for log matcher usage? (not obligatory, decided for
   yourself if it would help or just be noise)

Also things to consider...

4. Look into the matcher name.  Log seems fine, but just in case
   it may be useful to have another thought about it.  Trent suggested
   'description' in his issue1769.

5. Haddocks for this module (follow-up patch.)

6. Check the user manual to see if it needs updating.  (The manual says
   we have 6 primitive matchers whereas we would now have 8 with this
   patch, but are there other things to fix?).

Note for Trent
--------------
I thought darcs help patterns was the definitive guide to matchers,
but it turns out there's actually quite a bit we have to say about it
in the manual.  Does the content in the latter need to go away, move,
or is it fine where it is?

On Sun, Apr 25, 2010 at 21:09:53 +0000, Matthias Kilian wrote:
> Sun Apr 25 23:06:51 CEST 2010  Matthias Kilian <kili at outback.escape.de>
>   * Add support for matching against log messages

Grumpy old man challenge
------------------------
I think this passes, personally <http://wiki.darcs.net/Ideas>

1. What problem does the proposed feature solve?

   There is no way to look patches up on the basis of text that extends
   beyond the short name.

2. What are the user stories?

   Trent has some autogenerated patches, for example, by etckeeper that
   dump useful information in the long comment, for example, the
   packages installed.  He wants to find patches that refer to those
   packages, so he would say something like
   
   darcs changes --match 'log libarchive1 2.8.0-2'

3. Does this change any pre-existing workflows?  Does this introduce any
   incompatibilities?

   No pre-existing workflows changed.  It could generalise --match
   'name' which some people are already using.

4. How do we preserve the conceptual integrity of Darcs?
   Is the UI for this feature really Darcs-ish?

   This is absolutely a Darcs-ish way of doing things; we're just
   extending the pre-existing matcher framework, and we're not even
   doing anything usual in our extensions (like match conflicted).
   It's a simple case of picking something out of the metadata.

5. What are the possible unintended interactions with other pre-existing features?

   People might get used to --match 'log XXX' (because it's more
   general) and be confused by why -p does not do the same thing.

   People might be confused by 'log' vs 'description' (patch bundles) vs
   'long comment'.  Already not many people have this distinctions in
   their heads (even I forget it sometimes and mix up the two).  I don't
   think there is much we can do about it, except maybe spare a moment's
   thought to the matcher name.

   Help text for matchers may become unwieldy?  This may start a pattern
   of too-many-matchers?

6. What are the alternative approaches to solving the same problem?
   Why do we prefer this one?

   You could do something like darcs changes -v | grep, but then you
   would not get the benefit of interactivity.  Basically, we prefer
   this option because it fits within the current scheme of being
   able to refer to patches by one of their metadata fields.

7. Who are the stakeholders? Who is going to benefit/be affected by this
   feature: end-users, repo farms like patch-tag, darcs developers?

   This should affect command line end-users.  Maintainers are a good
   example of potential users.

Add support for matching against log messages
---------------------------------------------
> -                          just_name, just_author, repopatchinfo, RepoPatchInfo,
> -                          human_friendly, to_xml, pi_date, set_pi_date,
> -                          pi_date_string, pi_date_bytestring,
> +                          just_name, just_author, just_log, repopatchinfo,
> +			  RepoPatchInfo, human_friendly, to_xml, pi_date,
> +			  set_pi_date, pi_date_string, pi_date_bytestring,

Here's that indentation change

> hunk ./src/Darcs/Patch/Info.hs 118
>  just_author :: PatchInfo -> String
>  just_author =  metadataToString . _pi_author
>  
> +just_log :: PatchInfo -> String
> +just_log = unlines . map BC.unpack . _pi_log

What's the difference in spirit between just_foo and pi_foo?
It seems to be that just_foo is raw and pi_foo is processed,
(pi_log filters out ignore-this, but just_log does not)

Perhaps this module needs some haddocks in a follow-up patch?  Just a
general attitude of leaving the place better than you found it :-)

> + , ("log", "check a regular expression against the log message"
> +         , []
> +         , logmatch )

Why no example?  Redundant?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20100427/f195cc67/attachment.pgp>


More information about the darcs-users mailing list