I am /not/ screening this one for the moment because I think the current
behavior merits a bit of discussion. Another, perhaps better way to fix this
would be to introduce a change of behavior for --ask-deps so that, for
amend, it ignores the old dependencies instead of only adding to the
existing set. This would allow users to remove unwanted dependencies, which
is currently not possible except by unrecording and then recording again. If
we agree on this we should document the new behavior in the help text for
the amend command.

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

patch 0942856223e2ffbcc0f4b38070c34a4bdb432e15
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Jan 24 20:34:57 CET 2019
  * don't offer already depended on patches with --ask-deps
  
  This reverts an unintended behavior change introduced by the fix for
  issue2618: the patches we already explicitly depend on are offered again.
  This is done by adding the old dependencies to the anonymous patch before
  filtering out its dependencies.

Attachments

• don_t-offer-already-depended-on-patches-with-__ask_deps.dpatch
• patch-preview.txt
• unnamed

I wouldn't expect amend --ask-deps to silently drop the old 
dependencies, so I'd rather it not ignore the old dependencies.
I'm not sure what a good UI would be though, short of adding
a --remove-deps option which would seem quite heavyweight (though
perhaps --unrecord provides a good precedent).

Regardless of that, this patch in itself seems like a strict 
improvement on the previous situation.

> I wouldn't expect amend --ask-deps to silently drop the old 
> dependencies, so I'd rather it not ignore the old dependencies.
> I'm not sure what a good UI would be though, short of adding
> a --remove-deps option which would seem quite heavyweight (though
> perhaps --unrecord provides a good precedent).
> 
> Regardless of that, this patch in itself seems like a strict 
> improvement on the previous situation.

I agree. Wrt a better UI, I think your old comment in the code has it
right: the old dependencies should be offered but default to 'yes, keep
them'. As this needs some (I think quite deep) refactoring of the patch
selection we'll have to postpone that.

Okay, so I am screening it now and we can sort out possible 
improvements to behavior later.

> Wrt a better UI, I think your old comment in the code has it
> right: the old dependencies should be offered but default to
> 'yes, keep them'. As this needs some (I think quite deep)
> refactoring of the patch selection we'll have to postpone that.

Ah yes, I forgot that :-) Anyway, accepting this patch.

I am not happy about this. Not being able to remove a dependency is a
serious limitation IMO. It means you'll have to unrecord and re-record
--ask-deps.

A sort term solution may be to do two selection phases: first you are
offered the existing deps and can chose to remove any of them, then a
second selection where you are offered new patches to depend on. That
should not be too hard to implement.

No objections from me. I don't use --ask-deps that much so I don't
run into it in practice, but it's definitely a logical limitation.