darcs

Patch 1860 add --not-in-remote option to amend and rebase unsuspend

Title add --not-in-remote option to amend and rebase unsuspend
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2019-07-29.19:55:49 by bfrk, last changed 2019-08-06.11:00:03 by ganesh.

Files
File name Status Uploaded Type Edit Remove
patch-preview.txt bfrk, 2019-07-29.19:55:49 text/x-darcs-patch
patch-preview.txt bfrk, 2019-08-05.21:16:14 text/x-darcs-patch
refactor-withselectedpatchfromrepo.dpatch bfrk, 2019-07-29.19:55:49 application/x-darcs-patch
rename-withselectedpatchfromrepo-to-withselectedpatchfromlist.dpatch bfrk, 2019-08-05.21:16:14 application/x-darcs-patch
unnamed bfrk, 2019-07-29.19:55:49 text/plain
unnamed bfrk, 2019-08-05.21:16:14 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21014 (view) Author: bfrk Date: 2019-07-29.19:55:49
3 patches for repository http://darcs.net/screened:

patch cc510af38b6b6bb1502c8e3ff0595706c79b9b27
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jul 19 10:12:52 CEST 2019
  * refactor withSelectedPatchFromRepo
  
  It now gets an RL of patches instead of a Repository. At the place of use in
  the amend command we do the readRepo and patchSet2RL ourselves. This
  prepares for adding --not-in-remote option to amend but makes sense
  independently.

patch 36779a9fba5df83b62465b8c095538ef88e2fc7c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jul 19 11:18:19 CEST 2019
  * add --not-in-remote option to amend and rebase unsuspend

patch 4b86c0457dad83032ce2841af6819a35d941135a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jul 19 16:37:57 CEST 2019
  * factor two functions used for --not-in-remote into Darcs.Util.English
Attachments
msg21016 (view) Author: bfrk Date: 2019-07-29.20:09:47
I have wanted this for a long time to make sure I don't accidentally 
edit upstream patches.

In fact, we may want this option to be on by default (for the 
defaultrepo, if there is one), for all commands that edit or remove 
existing patches. This needs a re-working of the option (UI and 
implementation) but that may be worthwhile.
msg21031 (view) Author: ganesh Date: 2019-08-05.14:45:12
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jul 19 10:12:52 CEST 2019
>   * refactor withSelectedPatchFromRepo
>   
>   It now gets an RL of patches instead of a Repository. At the 
>   place of use in
>   the amend command we do the readRepo and patchSet2RL ourselves. 
>   This
>   prepares for adding --not-in-remote option to amend but makes
>   sense independently.

OK. Perhaps it should be renamed to something like 
'withSelectedPatchFromList'.

> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jul 19 11:18:19 CEST 2019
>   * add --not-in-remote option to amend and rebase unsuspend

The patch description is wrong, it's actually for "rebase suspend" 
(which makes much more sense).

I guess that 'preselectPatches' could be factored out into something 
more central than D.UI.C.Unrecord.

It's unfortunate that we can't share the code for --not-in-remote 
between amend and obliterate/rebase suspend, but not surprising 
given that amend is single-select and the others are multi-select.

> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jul 19 16:37:57 CEST 2019
>   * factor two functions used for --not-in-remote into 
>     Darcs.Util.English

OK
msg21032 (view) Author: bfrk Date: 2019-08-05.15:36:58
>>   * refactor withSelectedPatchFromRepo
>>   
>>   It now gets an RL of patches instead of a Repository.
> 
> OK. Perhaps it should be renamed to something like 
> 'withSelectedPatchFromList'.

Yes. Will do.

>> Author: Ben Franksen <ben.franksen@online.de>
>> Date:   Fri Jul 19 11:18:19 CEST 2019
>>   * add --not-in-remote option to amend and rebase unsuspend
> 
> The patch description is wrong, it's actually for "rebase suspend" 
> (which makes much more sense).

Right, sorry about that.

> I guess that 'preselectPatches' could be factored out into something 
> more central than D.UI.C.Unrecord.

Yes, we have D.UI.C.Util for stuff like that. Will do.

> It's unfortunate that we can't share the code for --not-in-remote 
> between amend and obliterate/rebase suspend, but not surprising 
> given that amend is single-select and the others are multi-select.

Whether we select a single patch or many should be configurable instead
of requiring a different call. Refactoring this so that we have less
code duplication is also on my long-term todo list.
msg21034 (view) Author: bfrk Date: 2019-08-05.21:16:14
Following up on review.

2 patches for repository http://darcs.net/screened:

patch 260bf5b2904147230b94d615c061ff7c7ee64304
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug  5 19:18:19 CEST 2019
  * rename withSelectedPatchFromRepo to withSelectedPatchFromList

patch 360fbca6d240c0be514cb26b21af6fddadf06a71
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug  5 21:34:23 CEST 2019
  * move preselectPatches and getLastPatches into Darcs.UI.Commands.Util
Attachments
History
Date User Action Args
2019-07-29 19:55:49bfrkcreate
2019-07-29 20:09:47bfrksetstatus: needs-screening -> needs-review
messages: + msg21016
2019-08-05 14:45:12ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg21031
2019-08-05 15:36:58bfrksetmessages: + msg21032
2019-08-05 21:16:14bfrksetfiles: + patch-preview.txt, rename-withselectedpatchfromrepo-to-withselectedpatchfromlist.dpatch, unnamed
messages: + msg21034
2019-08-06 11:00:03ganeshsetstatus: accepted-pending-tests -> accepted