[darcs-users] [patch222] Add support for matching against log messages [status=amend-requested]
kowey at darcs.net
Tue Apr 27 15:28:59 UTC 2010
[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
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,
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
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
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
Size: 197 bytes
Desc: Digital signature
More information about the darcs-users