darcs

Issue 1158 --match and --patch are silently mutually exclusive

Title --match and --patch are silently mutually exclusive
Priority bug Status needs-implementation
Milestone Resolved in
Superseder Nosy List alain91, darcs-devel, dmitry.kurochkin, galbolle, jaredj, kowey
Assigned To alain91
Topics Matchers, ProbablyEasy, UI

Created on 2008-10-23.13:17:24 by galbolle, last changed 2015-06-25.19:09:29 by alain91.

Messages
msg6396 (view) Author: galbolle Date: 2008-10-23.13:17:22
When using darcs pull --match xx --patch yy, darcs behaves like
darcs pull --match xx.

On the other hand, darcs pull --patch yy --match xx behaves like 
darcs pull --patch yy.

I would expect both commands to have the same result, that is 'and' the two
matches, or at least give a warning.
msg6397 (view) Author: dagit Date: 2008-10-23.13:31:28
On Thu, Oct 23, 2008 at 6:17 AM, Florent Becker <bugs@darcs.net> wrote:
>
> New submission from Florent Becker <florent.becker@ens-lyon.org>:
>
> When using darcs pull --match xx --patch yy, darcs behaves like
> darcs pull --match xx.
>
> On the other hand, darcs pull --patch yy --match xx behaves like
> darcs pull --patch yy.
>
> I would expect both commands to have the same result, that is 'and' the two
> matches, or at least give a warning.

Additionally I could see the user meaning 'or'.  This is a case where
what we probably want is to have a little query language.  Like how
the find command has switches like -o.

Hmm...This requires more thought that I want to put towards it at the moment.

Thanks for the bug report!

Jason
msg8083 (view) Author: kowey Date: 2009-08-11.00:16:24
I'll bet --match xx --match yy suffers the same way.

I think it's reasonable to change the behaviour so that --match xx --match yy is
equivalent to --match "X && Y"
(which generalises to the --match --patch case)

This sounds easy to fix.  I'd recommend starting by writing tests for --match xx
--match yy, --patch xx --match yy and --match xx --match yy, as well as three
matches.

Please shout if you disagree with the proposed UI
msg18535 (view) Author: alain91 Date: 2015-06-17.17:49:53
Could we say ?
1/ multiple "match" or multiple "patch" could be more efficiently 
handle within one REGEXP (with logical operator). The and/or have to 
be explicit in the REGEXP. => to fix the issue, just display a 
warning to avoid multiple "match" or "patch"
2/ at the end, we only have to handle mixed match/patch 
3/ the previous point have to be done for all commands with mixed 
match/patch
msg18539 (view) Author: bf Date: 2015-06-17.19:40:07
My take on this:

  --patch XXX is sugar for --match 'name XXX'
  --hash XXX is sugar for --match 'hash XXX'

These are both documented and quite obvious. This reduces the problem to
multiple --match options. I think it makes lots of sense to add:

  --match XXX --match YYY is sugar for --match 'XXX && YYY'

as proposed by kowey.

We should combine these transformations into a separate de-sugaring
phase prior to the actual patch matching code. This would allow us to
simplify the MatchFlag type as a first step toward (badly needed)
restructuring of the patch matching code.
msg18540 (view) Author: alain91 Date: 2015-06-17.19:48:41
I will have a look to implement code in the following way "combine these 
transformations into a separate de-sugaring phase prior to the actual 
patch matching code."



Le 17/06/2015 21:40, Ben Franksen a écrit :
> Ben Franksen <benjamin.franksen@helmholtz-berlin.de> added the comment:
>
> My take on this:
>
>    --patch XXX is sugar for --match 'name XXX'
>    --hash XXX is sugar for --match 'hash XXX'
>
> These are both documented and quite obvious. This reduces the problem to
> multiple --match options. I think it makes lots of sense to add:
>
>    --match XXX --match YYY is sugar for --match 'XXX && YYY'
>
> as proposed by kowey.
>
> We should combine these transformations into a separate de-sugaring
> phase prior to the actual patch matching code. This would allow us to
> simplify the MatchFlag type as a first step toward (badly needed)
> restructuring of the patch matching code.
>
> __________________________________
> Darcs bug tracker <bugs@darcs.net>
> <http://bugs.darcs.net/issue1158>
> __________________________________
msg18568 (view) Author: bf Date: 2015-06-21.00:03:57
I am no longer certain this is a good idea (regarding --match X --match
Y as sugar for match 'X && Y'). The reason is that this covers only one
special case of combining matching options. There are many others, some
of which naturally combine and some of which really make no sense when
combined. Let me distinguish 4 groups of matching options: --from-x,
--to-x, --x (singular), and --xs (plural). I think we should allow at
most one option from each group. This more or less reflects what Darcs
is (and has been) doing all the time. The problem is that Darcs does not
complain but arbitrarily chooses one of the options, ignoring the other
one. Try for instance 'darcs log --tags=. --patches=1829' in the darcs
repo: no matter how you re-order the options, the --tags option gets
ignored. Properly fixing this needs some (re-)design work. Writing up a
consistent proposal has been on my TODO list for some time.

Anyone interested in this should take a hard look at
Darcs.UI.Options.Matching where all matching options are defined and
where the (currently) allowed combinations are listed.
msg18573 (view) Author: alain91 Date: 2015-06-21.09:46:31
I'm interesting to go further.

"we should allow at most one option from each group" could be to 
restrictive. I propose to go in more details to identify valid 
combinations of options.
The beginning is :
1/ all options must appear only once in the command line
2/ Darcs check valid combinations of options
3/ Darcs raises an error when encounters not valid combination
msg18614 (view) Author: alain91 Date: 2015-06-23.20:33:02
4 groups of matching options: --from-x, --to-x, --x (singular), and 
--xs (plural)

1/ hash (singular) exists but not hashes (plural). Is it OK ?
2/ tag and tags come to the same options OneTag in module 
Darcs.UI.Options.Matching. Is it OK ?
3/ Do we considere an implicit "and" between each group ?
4/ if not, add --or --and --not flags. But after the next problem is 
how to manage precedence without parenthesis ?
5/ Could we add a syntactic suggar for "tag" <=> match "tag XXX" ?
msg18615 (view) Author: bf Date: 2015-06-23.22:52:40
> 1/ hash (singular) exists but not hashes (plural). Is it OK ?

For hashes the match must always be exact (that's why you would use a
hash in the first place), so yes, plural makes no sense IMO.

2/ tag and tags come to the same options OneTag in module 
Darcs.UI.Options.Matching. Is it OK ?

I don't like it but this is how it works ATM. It is inconsistent at best
and misleading at worst. If it were my decision I would demote --tags to
an undocumented compatibility alias for --tag.

The real problem with --tag is that it is not an exact match by default.
I still think that regular expression matching for --tag is useless and
error prone. I have previously proposed a change in the UI but it was
voted down (IIRC) with the argument that the UI is more consistent as it is.

(To debunk the consistency argument: whenever I explain to casual darcs
users that --tag 2.1 is not enough you should use --tag '^2.1$' if you
want *exactly* version 2.1 because it might match the tag '2.1.1', they
shake their heads and stare at me in disbelief.)

3/ Do we considere an implicit "and" between each group ?

The groups I mentioned are not yet well-defined enough to make any
definite proposal based on them. There is --last and --index and so on
which do not fit this picture. Anyway, I think the general idea /
direction should go towards making all matching options mutually
exclusive. If you want to combine matchers, you can use a single match
with an elaborate match expression. That's what the --match etc are made
for.

4/ if not, add --or --and --not flags. But after the next problem is 
how to manage precedence without parenthesis ?

Exactly. And --match already supports all this. No need to repeat this
on the command line where we don't have the parsing capabilities etc.

5/ Could we add a syntactic suggar for "tag" <=> match "tag XXX" ?

It already is (AFAIK), no need to add anything.
History
Date User Action Args
2008-10-23 13:17:24galbollecreate
2008-10-23 13:31:30dagitsetstatus: unread -> unknown
nosy: kowey, dagit, simon, thorkilnaur, dmitry.kurochkin, galbolle
messages: + msg6397
2009-08-10 23:48:51adminsetnosy: - dagit
2009-08-11 00:10:35koweysettopic: + UI
nosy: kowey, simon, thorkilnaur, dmitry.kurochkin, galbolle
2009-08-11 00:16:32koweysetstatus: unknown -> needs-implementation
nosy: + jaredj
topic: + ProbablyEasy
messages: + msg8083
2009-08-18 00:25:38koweysettopic: + Matchers
nosy: kowey, simon, thorkilnaur, jaredj, dmitry.kurochkin, galbolle
2009-08-25 17:31:48adminsetnosy: + darcs-devel, - simon
2009-08-27 09:01:22koweysetnosy: kowey, darcs-devel, thorkilnaur, jaredj, dmitry.kurochkin, galbolle
2009-08-27 14:25:54adminsetnosy: kowey, darcs-devel, thorkilnaur, jaredj, dmitry.kurochkin, galbolle
2010-03-31 13:12:02koweysetnosy: - thorkilnaur
2015-06-16 21:32:29alain91setassignedto: alain91
nosy: + alain91
2015-06-17 17:47:49alain91setmessages: + msg18534
2015-06-17 17:48:44alain91setmessages: - msg18534
2015-06-17 17:49:54alain91setmessages: + msg18535
2015-06-17 19:40:09bfsetmessages: + msg18539
2015-06-17 19:48:42alain91setmessages: + msg18540
2015-06-21 00:03:58bfsetmessages: + msg18568
2015-06-21 09:46:33alain91setmessages: + msg18573
2015-06-23 20:33:03alain91setmessages: + msg18614
2015-06-23 22:52:41bfsetassignedto: alain91 -> (no value)
messages: + msg18615
2015-06-25 19:09:29alain91setassignedto: alain91