1 patch for repository http://darcs.net:

Sun Apr 25 23:06:51 CEST 2010  Matthias Kilian <kili@outback.escape.de>
  * Add support for matching against log messages

Attachments

• add-support-for-matching-against-log-messages.dpatch
• unnamed

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@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

On Tue, Apr 27, 2010 at 04:28:59PM +0100, Eric Kow wrote:
> Requests
> --------
> 1. Please remove the tabs in the new import lines.

Done. I'll send out the amended patch soon.

>    Perhaps you could persuade your text editor to switch to spaces-only
>    mode for Haskell code?

No, but I smell a new feature here ;-) add some repository preference
that allows for checking sources against some regexps and add a big
red marker when using darcs whatsnew (similar to what darcs already
does with trailing whitespace).

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

Changed (and I think it matches the feature request from twb).

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

Well, I added one, but I don't find those usage examples very useful.
YMMV.

> 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.

Changed.

> 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?).

I'll send a second patch that adds documentation for the two missing
matchers (hunk and description).

Ciao,
	Kili

1 patch for repository http://darcs.net:

Sun May  2 12:50:47 CEST 2010  Matthias Kilian <kili@outback.escape.de>
  * resolve issue1769: add support for --match 'description ...'

Attachments

• resolve-issue1769_-add-support-for-__match-_description-____.dpatch
• unnamed

1 patch for repository http://darcs.net:

Sun May  2 12:59:32 CEST 2010  Matthias Kilian <kili@outback.escape.de>
  * Documentation for `hunk' and `description' matchers
  While here, fix the usage example for the hunk matcher (add missing
  quotes).

Attachments

• documentation-for-_hunk_-and-_description_-matchers.dpatch
• unnamed

On Sun, May 02, 2010 at 12:04:42 +0000, Matthias Kilian wrote:
> > 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.
> 
> Changed.

Argh, I just thought of something!

The problem with 'description' is that darcs send uses 'description'
to refer to the patch bundle description; whereas what we're looking
for is what record calls the 'long comment'

What do you think we should do?  Call the matcher comment?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

On Sun, May 02, 2010 at 01:29:14PM +0100, Eric Kow wrote:
> Argh, I just thought of something!
> 
> The problem with 'description' is that darcs send uses 'description'
> to refer to the patch bundle description; whereas what we're looking
> for is what record calls the 'long comment'
> 
> What do you think we should do?  Call the matcher comment?

Sounds reasonable. If noone comes with a better suggestion, I'll
send a new version in an hour. (I'll also send a amended version
of my documentation patch, since it's telling lies about newlines).


Ciao,
	Kili

Hi all,

Op zondag 02 mei 2010 14:29 schreef Eric Kow:
> Eric Kow <kowey@darcs.net> added the comment:
> On Sun, May 02, 2010 at 12:04:42 +0000, Matthias Kilian wrote:
> > > 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.
> 
> Argh, I just thought of something!
> 
> The problem with 'description' is that darcs send uses 'description'
> to refer to the patch bundle description; whereas what we're looking
> for is what record calls the 'long comment'

Or the 'log'. Or the 'description'

The code that deals with this, i.e. Darcs.Patch.Info, only speaks of 'name' 
and 'log'. The words 'description' and 'comment' are not used there. The patch 
name is not part of the log.

In the online help of darcs record, 'comment' (in the option names), 'log' (in 
the --logfile option) and 'description' (in most of the running text) are used 
to denote what the Darcs.Patch.Info code calls the log.

> What do you think we should do?  Call the matcher comment?

If we'd want to settle on a single term, I vote for "comment". "description" 
can then be used for patch bundle descriptions, and "log" makes me, as a 
programmer, think of the log files where administrators can see what a program 
is doing.

Reinier

On Sun, May 02, 2010 at 12:08:51PM +0000, Matthias Kilian wrote:
> Subject: [patch222] Documentation for `hunk' and `description' matchers
           ^^^^^^^^^^
Huh? Has this been added by the bug tracker? I only used darcs send
--in-reply-to but didn't mention patch222 anywhere.

Anyway, disregard this patch. As noted, it's not correct and I'll
send a new one later.

1 patch for repository http://darcs.net:

Sun May  2 14:41:25 CEST 2010  Matthias Kilian <kili@outback.escape.de>
  * resolve issue1769: add support for --match 'comment ...'
  This uses `comment' rather than `description' as the latter term is
  already used by darcs send. (Pointed out by kowey@darcs.net)

Attachments

• resolve-issue1769_-add-support-for-__match-_comment-____.dpatch
• unnamed

resolve issue1769: add support for --match 'comment ...'
--------------------------------------------------------
> +just_log :: PatchInfo -> String
> +just_log = unlines . map BC.unpack . _pi_log

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

> +logmatch l (Sealed2 hp) = isJust $ matchRegex (mkRegex l) $ just_log (info hp)

Looks good to me.  Thanks for the patient amending.

I'm going to follow up with some semi-automated camel casing.  Yeah, I
know we should probably do the camels as one big bang and get it over
with.  Somehow, the trickle approach is all I can muster.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

On Sun, May 02, 2010 at 15:34:28 +0200, Matthias Kilian wrote:
> On Sun, May 02, 2010 at 12:08:51PM +0000, Matthias Kilian wrote:
> > Subject: [patch222] Documentation for `hunk' and `description' matchers
>            ^^^^^^^^^^
> Huh? Has this been added by the bug tracker? I only used darcs send
> --in-reply-to but didn't mention patch222 anywhere.

I conjecture that roundup is clever enough to figure out what ticket
you're talking about on the basis of your In-Reply-To header.

Anyway, the easiest way to send amendments with our patch tracker is

  darcs send --subject '[patch222]'

Although for other mail-based teams, --in-reply-to is a pretty good
way too.  See http://wiki.darcs.net/Development/PatchTracker

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-ae26333ce3affbe02046b2dac0afa1d9bceb1293

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-ae26333ce3affbe02046b2dac0afa1d9bceb1293