Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
On 22/02/2017 8:25 am, Augie Fackler wrote: Having thought about this more over the balance of my weekend, it makes sense that this is consistent with 'hg amend' from evolve, and it feels right enough. Since it wasn't previously documented we'll go ahead and do this, and if anyone complains we can revert it between now and 4.2 being final. I agree entirely! I'll push both patches with the commit message adjustments you pointed out upthread. Many thanks! Thank you greatly, also. Ben ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
On Mon, Feb 20, 2017 at 12:17 AM, Ben Schmidtwrote: > I recognise there are probably use cases for the current 'roll' > behaviour as well. What I'm primarily interested in is solving the > problems raised in the bug, so if we can do that another way, such as > adding a new action, that will satisfy me just fine. Having thought about this more over the balance of my weekend, it makes sense that this is consistent with 'hg amend' from evolve, and it feels right enough. Since it wasn't previously documented we'll go ahead and do this, and if anyone complains we can revert it between now and 4.2 being final. I'll push both patches with the commit message adjustments you pointed out upthread. Many thanks! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
It also adjusts and documents the new behaviour of 'roll'. It now fits nicely with the behaviour of 'commit --amend' and the 'edit' action, by discarding the date as well as the commit message of the second commit. Previously it used the later date, like 'fold', but this often wasn't desirable, for example, in the common use case of using 'roll' to add forgotten changes to a changeset (because 'hg add' was previously forgotten or not all changes were identified while using 'hg record'). This...could stand to be its own patch (generally any time a commit message contains the word "also" that's a sign you've really got two patches). I'm also not sure I'm sold: why shouldn't roll advance the date? I originally added another action, 'amnd' for 'amend', but then modifying 'roll' seemed equally appropriate, and with the benefit of not adding another action. For some back-story, see: https://bz.mercurial-scm.org/show_bug.cgi?id=4820 There was some brief discussion on this list a year ago: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083475.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083477.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083488.html https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-April/083489.html I recognise there are probably use cases for the current 'roll' behaviour as well. What I'm primarily interested in is solving the problems raised in the bug, so if we can do that another way, such as adding a new action, that will satisfy me just fine. Smiles, Ben diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -36,7 +36,7 @@ # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but combine it with the one above - # r, roll = like fold, but discard this commit's description + # r, roll = like fold, but discard this commit's description and date # d, drop = remove commit from history # m, mess = edit commit message without changing commit content # @@ -58,7 +58,7 @@ # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but combine it with the one above - # r, roll = like fold, but discard this commit's description + # r, roll = like fold, but discard this commit's description and date # d, drop = remove commit from history # m, mess = edit commit message without changing commit content # @@ -71,11 +71,11 @@ *** Add delta -Edit the commit message to your liking, then close the editor. For -this example, let's assume that the commit message was changed to -``Add beta and delta.`` After histedit has run and had a chance to -remove any old or temporary revisions it needed, the history looks -like this:: +Edit the commit message to your liking, then close the editor. The date used +for the commit will be the later of the two commits' dates. For this example, +let's assume that the commit message was changed to ``Add beta and delta.`` +After histedit has run and had a chance to remove any old or temporary +revisions it needed, the history looks like this:: @ 2[tip] 989b4d060121 2009-04-27 18:04 -0500 durin42 |Add beta and delta. @@ -97,9 +97,10 @@ allowing you to edit files freely, or even use ``hg record`` to commit some changes as a separate commit. When you're done, any remaining uncommitted changes will be committed as well. When done, run ``hg -histedit --continue`` to finish this step. You'll be prompted for a -new commit message, but the default commit message will be the -original message for the ``edit`` ed revision. +histedit --continue`` to finish this step. If there are uncommitted +changes, you'll be prompted for a new commit message, but the default +commit message will be the original message for the ``edit`` ed +revision, and the date of the original commit will be preserved. The ``message`` operation will give you a chance to revise a commit message without changing the contents. It's a shortcut for doing @@ -724,6 +725,15 @@ """ return True +def firstdate(self): +"""Returns true if the rule should preserve the date of the first +change. + +This exists mainly so that 'rollup' rules can be a subclass of +'fold'. +""" +return False + def finishfold(self, ui, repo, ctx, oldctx, newnode, internalchanges): parent = ctx.parents()[0].node() repo.ui.pushbuffer() @@ -742,7 +752,10 @@ [oldctx.description()]) + '\n' commitopts['message'] = newmessage # date -commitopts['date'] = max(ctx.date(), oldctx.date()) +if self.firstdate(): +commitopts['date'] = ctx.date() +else: +commitopts['date'] = max(ctx.date(), oldctx.date()) extra = ctx.extra().copy() # histedit_source # note: ctx is likely a temporary commit
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
> On Feb 20, 2017, at 12:00 AM, timelesswrote: > > Fwiw, recently someone identified that if's are roughly as expensive > as method calls. > > I /think/ that means it'd be better for: > +if self.firstdate(): > +commitopts['date'] = ctx.date() > +else: > +commitopts['date'] = max(ctx.date(), oldctx.date()) > > to be managed as: > > def date(self, ctx, oldctx): > return ctx.date() > > def date(self, ctx, oldctx): >return max(ctx.date(), oldctx.date()) Maybe. I’d do whatever’s architecturally more readable without concern for performance here, since it’s histedit and we’ll be doing this computation ~once a commit. The overhead of an extra function call is minor compared to the disk IO that implies. AF > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
Fwiw, recently someone identified that if's are roughly as expensive as method calls. I /think/ that means it'd be better for: +if self.firstdate(): +commitopts['date'] = ctx.date() +else: +commitopts['date'] = max(ctx.date(), oldctx.date()) to be managed as: def date(self, ctx, oldctx): return ctx.date() def date(self, ctx, oldctx): return max(ctx.date(), oldctx.date()) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
On 20/02/2017 12:39 pm, Augie Fackler wrote: On Feb 19, 2017, at 8:23 PM, Augie Facklerwrote: On Sun, Feb 19, 2017 at 02:51:46PM +1100, Ben Schmidt wrote: # HG changeset patch # User Ben Schmidt # Date 1487413828 -39600 # Sat Feb 18 21:30:28 2017 +1100 # Node ID 4037ff1c9713d73b21ddc182580eacacba254ea7 # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d histedit: improve documentation and behaviour of dates (issue4820) This clarifies in the histedit documentation that the 'edit' action preserves the date and that the 'fold' action uses the later date. The documentation was previously silent on this issue which left users in doubt. Wow, that's a great fix. I’m excited enough about this that I’ve done the split, see: https://hg.durin42.com/hg-wip/log?rev=only%28histedit%29 With your permission, I’d like to just take that first patch (the documentation fix), and then we can discuss the behavior change to `roll` on the list as a followup. Sound good? Yes, sorry I didn't split it. I did actually think of it, but about an hour after I emailed the patch off. Your split looks pretty good except for the commit messages. It's really the alteration of the rollup behaviour that addresses issue4820, and of course the description of that part of the change belongs with that commit, but it's currently at the bottom of the commit for the doc change. But yes, feel free to go ahead and land that part, and we can continue discussion of the other part. Smiles, Ben ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
> On Feb 19, 2017, at 8:23 PM, Augie Facklerwrote: > > On Sun, Feb 19, 2017 at 02:51:46PM +1100, Ben Schmidt wrote: >> # HG changeset patch >> # User Ben Schmidt >> # Date 1487413828 -39600 >> # Sat Feb 18 21:30:28 2017 +1100 >> # Node ID 4037ff1c9713d73b21ddc182580eacacba254ea7 >> # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d >> histedit: improve documentation and behaviour of dates (issue4820) >> >> This clarifies in the histedit documentation that the 'edit' action preserves >> the date and that the 'fold' action uses the later date. The documentation >> was >> previously silent on this issue which left users in doubt. > > Wow, that's a great fix. I’m excited enough about this that I’ve done the split, see: https://hg.durin42.com/hg-wip/log?rev=only%28histedit%29 With your permission, I’d like to just take that first patch (the documentation fix), and then we can discuss the behavior change to `roll` on the list as a followup. Sound good? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: improve documentation and behaviour of dates (issue4820)
On Sun, Feb 19, 2017 at 02:51:46PM +1100, Ben Schmidt wrote: > # HG changeset patch > # User Ben Schmidt> # Date 1487413828 -39600 > # Sat Feb 18 21:30:28 2017 +1100 > # Node ID 4037ff1c9713d73b21ddc182580eacacba254ea7 > # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d > histedit: improve documentation and behaviour of dates (issue4820) > > This clarifies in the histedit documentation that the 'edit' action preserves > the date and that the 'fold' action uses the later date. The documentation was > previously silent on this issue which left users in doubt. Wow, that's a great fix. > It also adjusts and documents the new behaviour of 'roll'. It now fits nicely > with the behaviour of 'commit --amend' and the 'edit' action, by discarding > the > date as well as the commit message of the second commit. Previously it used > the > later date, like 'fold', but this often wasn't desirable, for example, in the > common use case of using 'roll' to add forgotten changes to a changeset > (because 'hg add' was previously forgotten or not all changes were identified > while using 'hg record'). This...could stand to be its own patch (generally any time a commit message contains the word "also" that's a sign you've really got two patches). I'm also not sure I'm sold: why shouldn't roll advance the date? > > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -36,7 +36,7 @@ > # p, pick = use commit > # e, edit = use commit, but stop for amending > # f, fold = use commit, but combine it with the one above > - # r, roll = like fold, but discard this commit's description > + # r, roll = like fold, but discard this commit's description and date > # d, drop = remove commit from history > # m, mess = edit commit message without changing commit content > # > @@ -58,7 +58,7 @@ > # p, pick = use commit > # e, edit = use commit, but stop for amending > # f, fold = use commit, but combine it with the one above > - # r, roll = like fold, but discard this commit's description > + # r, roll = like fold, but discard this commit's description and date > # d, drop = remove commit from history > # m, mess = edit commit message without changing commit content > # > @@ -71,11 +71,11 @@ > *** > Add delta > > -Edit the commit message to your liking, then close the editor. For > -this example, let's assume that the commit message was changed to > -``Add beta and delta.`` After histedit has run and had a chance to > -remove any old or temporary revisions it needed, the history looks > -like this:: > +Edit the commit message to your liking, then close the editor. The date used > +for the commit will be the later of the two commits' dates. For this example, > +let's assume that the commit message was changed to ``Add beta and delta.`` > +After histedit has run and had a chance to remove any old or temporary > +revisions it needed, the history looks like this:: > > @ 2[tip] 989b4d060121 2009-04-27 18:04 -0500 durin42 > |Add beta and delta. > @@ -97,9 +97,10 @@ > allowing you to edit files freely, or even use ``hg record`` to commit > some changes as a separate commit. When you're done, any remaining > uncommitted changes will be committed as well. When done, run ``hg > -histedit --continue`` to finish this step. You'll be prompted for a > -new commit message, but the default commit message will be the > -original message for the ``edit`` ed revision. > +histedit --continue`` to finish this step. If there are uncommitted > +changes, you'll be prompted for a new commit message, but the default > +commit message will be the original message for the ``edit`` ed > +revision, and the date of the original commit will be preserved. > > The ``message`` operation will give you a chance to revise a commit > message without changing the contents. It's a shortcut for doing > @@ -724,6 +725,15 @@ > """ > return True > > +def firstdate(self): > +"""Returns true if the rule should preserve the date of the first > +change. > + > +This exists mainly so that 'rollup' rules can be a subclass of > +'fold'. > +""" > +return False > + > def finishfold(self, ui, repo, ctx, oldctx, newnode, internalchanges): > parent = ctx.parents()[0].node() > repo.ui.pushbuffer() > @@ -742,7 +752,10 @@ > [oldctx.description()]) + '\n' > commitopts['message'] = newmessage > # date > -commitopts['date'] = max(ctx.date(), oldctx.date()) > +if self.firstdate(): > +commitopts['date'] = ctx.date() > +else: > +commitopts['date'] = max(ctx.date(), oldctx.date()) > extra = ctx.extra().copy() > # histedit_source > # note: ctx is likely a temporary commit but that the best we can do > @@ -809,7 +822,7 @@ > return True >
[PATCH] histedit: improve documentation and behaviour of dates (issue4820)
# HG changeset patch # User Ben Schmidt# Date 1487413828 -39600 # Sat Feb 18 21:30:28 2017 +1100 # Node ID 4037ff1c9713d73b21ddc182580eacacba254ea7 # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d histedit: improve documentation and behaviour of dates (issue4820) This clarifies in the histedit documentation that the 'edit' action preserves the date and that the 'fold' action uses the later date. The documentation was previously silent on this issue which left users in doubt. It also adjusts and documents the new behaviour of 'roll'. It now fits nicely with the behaviour of 'commit --amend' and the 'edit' action, by discarding the date as well as the commit message of the second commit. Previously it used the later date, like 'fold', but this often wasn't desirable, for example, in the common use case of using 'roll' to add forgotten changes to a changeset (because 'hg add' was previously forgotten or not all changes were identified while using 'hg record'). diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -36,7 +36,7 @@ # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but combine it with the one above - # r, roll = like fold, but discard this commit's description + # r, roll = like fold, but discard this commit's description and date # d, drop = remove commit from history # m, mess = edit commit message without changing commit content # @@ -58,7 +58,7 @@ # p, pick = use commit # e, edit = use commit, but stop for amending # f, fold = use commit, but combine it with the one above - # r, roll = like fold, but discard this commit's description + # r, roll = like fold, but discard this commit's description and date # d, drop = remove commit from history # m, mess = edit commit message without changing commit content # @@ -71,11 +71,11 @@ *** Add delta -Edit the commit message to your liking, then close the editor. For -this example, let's assume that the commit message was changed to -``Add beta and delta.`` After histedit has run and had a chance to -remove any old or temporary revisions it needed, the history looks -like this:: +Edit the commit message to your liking, then close the editor. The date used +for the commit will be the later of the two commits' dates. For this example, +let's assume that the commit message was changed to ``Add beta and delta.`` +After histedit has run and had a chance to remove any old or temporary +revisions it needed, the history looks like this:: @ 2[tip] 989b4d060121 2009-04-27 18:04 -0500 durin42 |Add beta and delta. @@ -97,9 +97,10 @@ allowing you to edit files freely, or even use ``hg record`` to commit some changes as a separate commit. When you're done, any remaining uncommitted changes will be committed as well. When done, run ``hg -histedit --continue`` to finish this step. You'll be prompted for a -new commit message, but the default commit message will be the -original message for the ``edit`` ed revision. +histedit --continue`` to finish this step. If there are uncommitted +changes, you'll be prompted for a new commit message, but the default +commit message will be the original message for the ``edit`` ed +revision, and the date of the original commit will be preserved. The ``message`` operation will give you a chance to revise a commit message without changing the contents. It's a shortcut for doing @@ -724,6 +725,15 @@ """ return True +def firstdate(self): +"""Returns true if the rule should preserve the date of the first +change. + +This exists mainly so that 'rollup' rules can be a subclass of +'fold'. +""" +return False + def finishfold(self, ui, repo, ctx, oldctx, newnode, internalchanges): parent = ctx.parents()[0].node() repo.ui.pushbuffer() @@ -742,7 +752,10 @@ [oldctx.description()]) + '\n' commitopts['message'] = newmessage # date -commitopts['date'] = max(ctx.date(), oldctx.date()) +if self.firstdate(): +commitopts['date'] = ctx.date() +else: +commitopts['date'] = max(ctx.date(), oldctx.date()) extra = ctx.extra().copy() # histedit_source # note: ctx is likely a temporary commit but that the best we can do @@ -809,7 +822,7 @@ return True @action(["roll", "r"], -_("like fold, but discard this commit's description")) +_("like fold, but discard this commit's description and date")) class rollup(fold): def mergedescs(self): return False @@ -817,6 +830,9 @@ def skipprompt(self): return True +def firstdate(self): +return True + @action(["drop", "d"], _('remove commit from history')) class drop(histeditaction): @@ -884,11 +900,11 @@ - `mess` to reword the