Created on 2022-12-16.18:33:06 by gpiero, last changed 2023-05-28.09:15:42 by gpiero.
See mailing list archives
for discussion on individual patches.
msg23051 (view) |
Author: gpiero |
Date: 2022-12-16.18:33:05 |
|
Some very old (almost 6-year old) patches that I've just rebased against
screened. Should look at my todo list more often.
Not sure if the help text makes sense in English, looking for your judgement.
The rationale for 9b86a4f2b022bf1387d4d2d5c6c1a54abf7aa536 is that currently:
$ darcs log -h 5d6 --count
1
$ darcs log --matches 'hash 5d6' --count
5
While I don't really expect anyone to select a bunch of patches based on a
common hash prefix, this is for coherence and for honoring the contract with
the user.
3 patches for repository http://darcs.net/screened:
patch d682cdb483060d25b6921c75360c53cefe52c6eb
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date: Fri Dec 16 18:47:02 CET 2022
* clarify help for --hash, --from-hash and --to-hash options
patch edabbbd32a9cd4091e4dff2f4f909f15c1446d52
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date: Fri Dec 16 18:59:36 CET 2022
* mention --from-hash and --to-hash in the output of `darcs help patterns`
patch 9b86a4f2b022bf1387d4d2d5c6c1a54abf7aa536
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date: Fri Dec 16 19:07:43 CET 2022
* add --hashes option and use it instead of --hash where appropriate
This way, -h/--hash/--hashes really become aliases for
--match/--matches 'hash ...', as suggested by `darcs help patterns`.
Attachments
|
msg23052 (view) |
Author: gpiero |
Date: 2022-12-16.18:41:23 |
|
mmpf, really need to check I've recorded all the changes before sending
them...
Attachments
|
msg23071 (view) |
Author: ganesh |
Date: 2023-01-01.02:55:03 |
|
The changes make sense to me.
On the English, when you say "a patch whose hash prefix matches HASH" I
guess this is meant to be parsed as "a patch whose hash (prefix matches)
HASH"?
The tricky bit is that it probably more naturally parses as "a patch
whose (hash prefix) matches HASH" which sort of implies the existence of
a unique hash prefix.
I wonder if renaming the meta-variable and going for something like "a
patch whose hash starts with HASHPREFIX" would work better?
|
msg23080 (view) |
Author: bfrk |
Date: 2023-01-14.19:22:56 |
|
I agree that the last wording proposed by Ganesh is the best option
here.
Apart from that I am fine with the 3 patches. The code changes look
reasonable to me. I would feel safer with a test or two, and would
like to encourage you to add one, but this is not a requirement.
(That would be hypocritical, given that we don't have any systematic
tests for matching options yet.)
|
msg23084 (view) |
Author: gpiero |
Date: 2023-01-30.17:46:52 |
|
Updated bundle (with test).
Attachments
|
msg23093 (view) |
Author: bfrk |
Date: 2023-02-16.09:21:01 |
|
I see no tests being added. Have you forgot to add them? Otherwise
fine.
|
msg23094 (view) |
Author: bfrk |
Date: 2023-02-16.09:22:56 |
|
Can I mark issue2695 as resolved when this is pushed?
|
msg23105 (view) |
Author: bfrk |
Date: 2023-02-19.08:45:17 |
|
Pushed latest bundle to screened.
|
msg23115 (view) |
Author: bfrk |
Date: 2023-02-20.21:55:09 |
|
It does indeed resolve issue2695. Anyway, accepted.
|
msg23228 (view) |
Author: gpiero |
Date: 2023-03-29.15:49:12 |
|
* [Thu, Feb 16, 2023 at 09:21:06AM +0000] Ben Franksen:
>I see no tests being added. Have you forgot to add them? Otherwise
>fine.
It seems that the wrong bundle has been screened. More precisely, [0]
(wrong one) has been screened instead of [1] (right one).
In another thread Ganesh suggested to mark superseded bundles as dead.
Maybe deleting them causes less confusion (at the cost of losing some
history)?
Regarding these patches, can you check if [1] is OK?
Once that's done, let me know if you prefer to rollback the old ones and
apply the new ones (if that can be done cleanly), apply a fix-it-all
patch or simply let the things as they are.
* [Thu, Feb 16, 2023 at 09:22:57AM +0000] Ben Franksen:
>Can I mark issue2695 as resolved when this is pushed?
I thought there were a hook that automatically did this when a patch
name loosely matched 'resolve issuexxx', am I wrong?
Ciao,
Gian Piero.
[0]
http://bugs.darcs.net/file9566/clarify-help-for-__hash_-__from_hash-and-__to_hash-options.dpatch
[1]
http://bugs.darcs.net/file9575/accept-issue2695_-_log-__hash-x_-is-not-equivalent-to-_log-__matches-_hash-x__.dpatch
|
msg23229 (view) |
Author: bfrk |
Date: 2023-03-30.06:39:56 |
|
OMG, sorry for the mess! I am not sure how best to fix this. Ganesh do you
have a suggestion?
I personally prefer to obliterate the old bundle in screened. The only
persons affected are you, me, and Ganesh. Among us three we should be able to
manage. There may be lurkers but if they don't do any development of their
own they should be fine.
> I thought there were a hook that automatically did this when a patch
> name loosely matched 'resolve issuexxx', am I wrong?
It seems this automation was lost when we moved things to a new server. I
tried to restore it from the parts of the old machine that we archived but
could not find out how it was done. There are no hooks in the old repos and
nothing related to this kind of functionality in the old roundup
configuration. It is possible that it was instead triggered by the email
processing machinery but I have no idea how that works and where it is
configured.
|
msg23230 (view) |
Author: bfrk |
Date: 2023-03-30.09:07:16 |
|
Sorry for bringing it up so late in the process, but after thinking again about this
change I have serious doubts.
Yes, it makes the UI more consistent, which is a good thing per se. However, from a
practical perspective, I'd really prefer it if darcs would fail if the given hash
prefix is not unique (which is how it works in git, IIRC). When you give darcs a
hash, you are implicitly saying "I want precisely this (unique) patch". Why else
would you bother to use a hash match? This applies to the singular version of the
option; the plural version (--hashes) is useless IMO and should be removed. For match
expressions (the --*match* options) I would also want uniqueness for every 'hash'
term; the result can still be multiple patches e.g. if you use 'hash xx or hash yy'.
This is related to my long-standing issue with tag matches: like for hashes, when you
match on a tag you usually want a unique match (when using the singular form of the
option). There may be the occasional exception but I'd rather have to use a work-
around in these few cases than the other way around.
One idea to solve the UI problem is to provide a --unique option, which would qualify
any match expression to express that the match must be unique or else fail. But --
unique doesn't make much sense for many of the allowed matches (why would you ever
want to get a unique match for date, author, or long comment?). It is also very
inefficient for anything that needs to access patch content i.e. hunk and touch. In
practice the only matches where you may want unique matches are: tag, hash, and name.
We could limit --unique to these three match types and fail if you try to use it with
any other match expression.
In the end I think this is too much complication for too little gain. We should
simply bite the bullet and require (and check) that (singular) 'tag' and 'hash'
matches are unique (and clearly document that). This is certainly the most useful
behavior in practice; and (IME) it is also the behavior naive users expect. Strictly
speaking this breaks compatibility, but I am almost sure nobody relies on the current
behavior for correctness.
(Side note: implementing a uniqueness queck probably requires some refactoring of the
matching machinery. Also, without some sort of index it will be linear in the number
of patches in the repo, which may turn out to be prohibitively expensive.)
|
msg23231 (view) |
Author: ganesh |
Date: 2023-03-30.13:19:42 |
|
> I personally prefer to obliterate the old bundle in screened. The
> only persons affected are you, me, and Ganesh. Among us three we
> should be able to manage. There may be lurkers but if they don't
> do any development of their own they should be fine.
I think there's also this mirror:
https://hub.darcs.net/darcs/darcs-screened
I'm fine with obliterating if that's what you prefer.
|
msg23233 (view) |
Author: bfrk |
Date: 2023-03-31.11:11:26 |
|
Marking as "in-discussion". I will obliterate the patches from
screened and the hub.darcs mirror (and also check reviewed just to
be sure).
@gpiero I would love to get your feedback regarding my reservations
and the counter-proposal.
|
msg23234 (view) |
Author: bfrk |
Date: 2023-03-31.11:31:42 |
|
@ganesh I seem to be unable to obliterate from
https://hub.darcs.net/darcs/darcs-screened. I tried
> ssh bf@hub.darcs.net obliterate darcs/darcs-screened
invalid repository name
> ssh darcs@hub.darcs.net obliterate darcs-screened
darcs@hub.darcs.net: Permission denied (publickey).
and as darcs-unstable@darcs.net:
> ssh darcs@hub.darcs.net obliterate darcs-screened
Unable to negotiate with 173.255.254.113 port 22: no matching cipher
found. Their offer: aes256-cbc,aes192-cbc,aes128-cbc
|
msg23278 (view) |
Author: gpiero |
Date: 2023-05-28.09:15:41 |
|
* [Thu, Mar 30, 2023 at 09:07:16AM +0000] Ben Franksen:
>In the end I think this is too much complication for too little gain.
>We should simply bite the bullet and require (and check) that
>(singular) 'tag' and 'hash' matches are unique (and clearly document
>that). This is certainly the most useful behavior in practice; and
>(IME) it is also the behavior naive users expect. Strictly speaking
>this breaks compatibility, but I am almost sure nobody relies on the
>current behavior for correctness.
As for 'hash', I think it's the best option, especially if darcs errors
out outputting a list of possible matches, like git does.
Regarding 'tag', I've have some instinctive doubts. I use darcs for
personal software and I don't have a need for tags, so I don't have a
strong opinion on this. But, for example, the use documented in
http://bugs.darcs.net/issue2702 seems legit. OTOH, I cannot think of a
single case in which a pattern match on the hash can be legit.
>(Side note: implementing a uniqueness queck probably requires some
>refactoring of the matching machinery. Also, without some sort of index
>it will be linear in the number of patches in the repo, which may turn
>out to be prohibitively expensive.)
I've had a cursory look and I have no idea how and where to hook it.
So, as usual for darcs things: yes, Ben, thank you. :)
|
|
Date |
User |
Action |
Args |
2022-12-16 18:33:06 | gpiero | create | |
2022-12-16 18:41:27 | gpiero | set | files:
+ clarify-help-for-__hash_-__from_hash-and-__to_hash-options.dpatch messages:
+ msg23052 title: clarify help for --hash, --from-hash and... (and 2 more) -> Re: darcs patch: clarify help for --hash, --from-hash and... (and 2 |
2023-01-01 02:55:04 | ganesh | set | status: needs-screening -> review-in-progress messages:
+ msg23071 |
2023-01-14 19:22:57 | bfrk | set | messages:
+ msg23080 |
2023-01-30 17:46:54 | gpiero | set | files:
+ accept-issue2695_-_log-__hash-x_-is-not-equivalent-to-_log-__matches-_hash-x__.dpatch messages:
+ msg23084 |
2023-02-16 09:21:06 | bfrk | set | messages:
+ msg23093 |
2023-02-16 09:22:57 | bfrk | set | messages:
+ msg23094 |
2023-02-19 08:45:19 | bfrk | set | messages:
+ msg23105 title: Re: darcs patch: clarify help for --hash, --from-hash and... (and 2 -> clarify help for --hash, --from-hash and... (and 2 more) |
2023-02-20 21:55:10 | bfrk | set | status: review-in-progress -> accepted-pending-tests messages:
+ msg23115 |
2023-03-29 15:49:13 | gpiero | set | messages:
+ msg23228 title: clarify help for --hash, --from-hash and... (and 2 more) -> Re: darcs patch: clarify help for --hash, --from-hash and... (and 2 |
2023-03-30 06:40:06 | bfrk | set | messages:
+ msg23229 |
2023-03-30 09:07:16 | bfrk | set | messages:
+ msg23230 |
2023-03-30 13:19:43 | ganesh | set | messages:
+ msg23231 |
2023-03-31 11:11:26 | bfrk | set | status: accepted-pending-tests -> in-discussion messages:
+ msg23233 |
2023-03-31 11:31:43 | bfrk | set | messages:
+ msg23234 |
2023-05-28 09:15:42 | gpiero | set | messages:
+ msg23278 |
|