| Submitter | Mateusz Kwapich |
|---|---|
| Date | Feb. 18, 2015, 12:52 a.m. |
| Message ID | <494a60ee7b222940db7f.1424220738@dev1429.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/7804/ |
| State | Superseded |
| Commit | 5a64b676c5d3f3a0692b7fa1414e702f7231e133 |
| Headers | show |
Comments
On 2/17/2015 4:52 PM, Mateusz Kwapich wrote: > # HG changeset patch > # User Mateusz Kwapich <mitrandir@fb.com> > # Date 1421952770 28800 > # Thu Jan 22 10:52:50 2015 -0800 > # Node ID 494a60ee7b222940db7f95c0546ada91d13e180c > # Parent ff5caa8dfd993680d9602ca6ebb14da9de10d5f4 > histedit: extract method ruleeditor > > Extract functionality of editing histedit rules to separate method so we can > reuse it in upcoming --edit-plan option. > > diff --git a/hgext/histedit.py b/hgext/histedit.py > --- a/hgext/histedit.py > +++ b/hgext/histedit.py > @@ -638,16 +638,9 @@ > > ctxs = [repo[r] for r in revs] > if not rules: > - rules = '\n'.join([makedesc(c) for c in ctxs]) > - rules += '\n\n' > - rules += editcomment % (node.short(root), node.short(topmost)) > - rules = ui.edit(rules, ui.username()) > - # Save edit rules in .hg/histedit-last-edit.txt in case > - # the user needs to ask for help after something > - # surprising happens. > - f = open(repo.join('histedit-last-edit.txt'), 'w') > - f.write(rules) > - f.close() > + rules = ruleeditor(repo, ui, [['pick', c] for c in ctxs], > + editcomment=editcomment % (node.short(root), > + node.short(topmost))) > else: > if rules == '-': > f = sys.stdin > @@ -805,20 +798,40 @@ > raise util.Abort(_('cannot edit immutable changeset: %s') % root) > return [c.node() for c in ctxs] > > -def makedesc(c): > - """build a initial action line for a ctx `c` > +def makedesc(repo, action, rev): > + """build a initial action line for a ctx > > line are in the form: > > - pick <hash> <rev> <summary> > + <action> <hash> <rev> <summary> > """ > + ctx = repo[rev] > summary = '' > - if c.description(): > - summary = c.description().splitlines()[0] > - line = 'pick %s %d %s' % (c, c.rev(), summary) > + if ctx.description(): > + summary = ctx.description().splitlines()[0] > + line = '%s %s %d %s' % (action, ctx, ctx.rev(), summary) > # trim to 80 columns so it's not stupidly wide in my editor > return util.ellipsis(line, 80) The simultaneous changes to makedesc make this diff harder to parse. Could you split those out into their own change? That will make this be two trivial refactors instead of one more difficult refactor. > > +def ruleeditor(repo, ui, rules, editcomment=""): > + """open an editor to edit rules > + > + rules are in the format [ [act, ctx], ...] like in state.rules > + """ > + rules = '\n'.join([makedesc(repo, act, rev) for [act, rev] in rules]) > + rules += '\n\n' > + rules += editcomment > + rules = ui.edit(rules, ui.username()) > + > + # Save edit rules in .hg/histedit-last-edit.txt in case > + # the user needs to ask for help after something > + # surprising happens. > + f = open(repo.join('histedit-last-edit.txt'), 'w') > + f.write(rules) > + f.close() > + > + return rules > + > def verifyrules(rules, repo, ctxs): > """Verify that there exists exactly one edit rule per given changeset. > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Wed, Feb 18, 2015 at 10:48:40AM -0800, Ryan McElroy wrote: > On 2/17/2015 4:52 PM, Mateusz Kwapich wrote: > ># HG changeset patch > ># User Mateusz Kwapich <mitrandir@fb.com> > ># Date 1421952770 28800 > ># Thu Jan 22 10:52:50 2015 -0800 > ># Node ID 494a60ee7b222940db7f95c0546ada91d13e180c > ># Parent ff5caa8dfd993680d9602ca6ebb14da9de10d5f4 > >histedit: extract method ruleeditor > > > >Extract functionality of editing histedit rules to separate method so we can > >reuse it in upcoming --edit-plan option. > > > >diff --git a/hgext/histedit.py b/hgext/histedit.py > >--- a/hgext/histedit.py > >+++ b/hgext/histedit.py > >@@ -638,16 +638,9 @@ > > ctxs = [repo[r] for r in revs] > > if not rules: > >- rules = '\n'.join([makedesc(c) for c in ctxs]) > >- rules += '\n\n' > >- rules += editcomment % (node.short(root), node.short(topmost)) > >- rules = ui.edit(rules, ui.username()) > >- # Save edit rules in .hg/histedit-last-edit.txt in case > >- # the user needs to ask for help after something > >- # surprising happens. > >- f = open(repo.join('histedit-last-edit.txt'), 'w') > >- f.write(rules) > >- f.close() > >+ rules = ruleeditor(repo, ui, [['pick', c] for c in ctxs], > >+ editcomment=editcomment % (node.short(root), > >+ node.short(topmost))) > > else: > > if rules == '-': > > f = sys.stdin > >@@ -805,20 +798,40 @@ > > raise util.Abort(_('cannot edit immutable changeset: %s') % root) > > return [c.node() for c in ctxs] > >-def makedesc(c): > >- """build a initial action line for a ctx `c` > >+def makedesc(repo, action, rev): > >+ """build a initial action line for a ctx > > line are in the form: > >- pick <hash> <rev> <summary> > >+ <action> <hash> <rev> <summary> > > """ > >+ ctx = repo[rev] > > summary = '' > >- if c.description(): > >- summary = c.description().splitlines()[0] > >- line = 'pick %s %d %s' % (c, c.rev(), summary) > >+ if ctx.description(): > >+ summary = ctx.description().splitlines()[0] > >+ line = '%s %s %d %s' % (action, ctx, ctx.rev(), summary) > > # trim to 80 columns so it's not stupidly wide in my editor > > return util.ellipsis(line, 80) > The simultaneous changes to makedesc make this diff harder to parse. Could > you split those out into their own change? That will make this be two > trivial refactors instead of one more difficult refactor. +1, agree with Ryan's comments enough I'm dropping these and will wait for a resend. > > >+def ruleeditor(repo, ui, rules, editcomment=""): > >+ """open an editor to edit rules > >+ > >+ rules are in the format [ [act, ctx], ...] like in state.rules > >+ """ > >+ rules = '\n'.join([makedesc(repo, act, rev) for [act, rev] in rules]) > >+ rules += '\n\n' > >+ rules += editcomment > >+ rules = ui.edit(rules, ui.username()) > >+ > >+ # Save edit rules in .hg/histedit-last-edit.txt in case > >+ # the user needs to ask for help after something > >+ # surprising happens. > >+ f = open(repo.join('histedit-last-edit.txt'), 'w') > >+ f.write(rules) > >+ f.close() > >+ > >+ return rules > >+ > > def verifyrules(rules, repo, ctxs): > > """Verify that there exists exactly one edit rule per given changeset. > >_______________________________________________ > >Mercurial-devel mailing list > >Mercurial-devel@selenic.com > >http://selenic.com/mailman/listinfo/mercurial-devel > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -638,16 +638,9 @@ ctxs = [repo[r] for r in revs] if not rules: - rules = '\n'.join([makedesc(c) for c in ctxs]) - rules += '\n\n' - rules += editcomment % (node.short(root), node.short(topmost)) - rules = ui.edit(rules, ui.username()) - # Save edit rules in .hg/histedit-last-edit.txt in case - # the user needs to ask for help after something - # surprising happens. - f = open(repo.join('histedit-last-edit.txt'), 'w') - f.write(rules) - f.close() + rules = ruleeditor(repo, ui, [['pick', c] for c in ctxs], + editcomment=editcomment % (node.short(root), + node.short(topmost))) else: if rules == '-': f = sys.stdin @@ -805,20 +798,40 @@ raise util.Abort(_('cannot edit immutable changeset: %s') % root) return [c.node() for c in ctxs] -def makedesc(c): - """build a initial action line for a ctx `c` +def makedesc(repo, action, rev): + """build a initial action line for a ctx line are in the form: - pick <hash> <rev> <summary> + <action> <hash> <rev> <summary> """ + ctx = repo[rev] summary = '' - if c.description(): - summary = c.description().splitlines()[0] - line = 'pick %s %d %s' % (c, c.rev(), summary) + if ctx.description(): + summary = ctx.description().splitlines()[0] + line = '%s %s %d %s' % (action, ctx, ctx.rev(), summary) # trim to 80 columns so it's not stupidly wide in my editor return util.ellipsis(line, 80) +def ruleeditor(repo, ui, rules, editcomment=""): + """open an editor to edit rules + + rules are in the format [ [act, ctx], ...] like in state.rules + """ + rules = '\n'.join([makedesc(repo, act, rev) for [act, rev] in rules]) + rules += '\n\n' + rules += editcomment + rules = ui.edit(rules, ui.username()) + + # Save edit rules in .hg/histedit-last-edit.txt in case + # the user needs to ask for help after something + # surprising happens. + f = open(repo.join('histedit-last-edit.txt'), 'w') + f.write(rules) + f.close() + + return rules + def verifyrules(rules, repo, ctxs): """Verify that there exists exactly one edit rule per given changeset.