D2013: commit: allow --no-secret to override phases.new-commit setting

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In https://phab.mercurial-scm.org/D2013#34509, @martinvonz wrote:
  
  > In https://phab.mercurial-scm.org/D2013#34320, @spectral wrote:
  >
  > > In https://phab.mercurial-scm.org/D2013#33867, @martinvonz wrote:
  > >
  > > > I wonder if we should instead have a --draft option for this. Reasons:
  > > >
  > > > - If we ever add a fourth phase (like Jun's proposed "archived" phase), then --no-secret doesn't clearly indicate "draft", it could just as well be "archived".
  > > > - Actually, we of course already do have a third phase. One could imagine a "hg commit --public", although that's probably not useful enough to warrant its own option, but it seems to suggest that "--no-secret" doesn't necessarily mean "draft".
  > > > - I find this tri-state boolean weird. "--secret" kind of defaults to off, but it can be made "more off" with "--no-secret".
  > >
  > >
  > > Yeah, I wasn't sure I liked it when writing it, and I'm fine with changing it, but do we really want a proliferation of flags?  Perhaps we want a generic 'phase' flag, so one can specify -s or --secret (BC and it's the "most common" case), and --phase <public|draft|secret|archived> for more advanced use-cases?  This runs into a couple small problems, specifically that there's now "more than one way to do it [specify secret]", it's a lot of typing, and I don't think we should abbreviate it `-p` (it just doesn't feel like it's going to be common enough to warrant any abbreviation, let alone 'p', which could stand for phase, or patch, or path, or a number of other words that we might eventually add to `commit` and be sad about not being able to abbreviate in the obvious fashion)
  >
  >
  > I think "--phase <phase>" makes sense, but it is a little long as you said. Since the real goal of this series is to preserve the phase on `hg split`, I'll see if I can do that by adding general support to scmutil.cleanupnodes() instead (Kyle and I already talked about this offline).
  
  
  Here's an update on this work:
  
  It turned out to be harder than I expected. The main problem seems to be that interrupted `hg rebase/histedit` does not call cleanupnodes, but they still expect the phase to get set. They also currently don't create obsmarkers. The reason they don't is because obsmarkers can't be undone, which is important for `hg rebase/histedit --abort`. However, these days repair.py will strip obsmarkers, so my plan now is this:
  
  1. Make rebase/histedit add obsmarkers as we go and undo them on abort [1]. This means that the user will see which commits have already been rebased when rebase/histedit is interrupted. I think that's a good thing.
  2. Make cleanupnodes() propagate phase to successor (without affecting parent commit, of course, so if parent is secret, then so will the successor be, even if the precursor was draft). This will be some new version of cleanupnodes() that will not strip the precursors in the obsmarker-less case.
  3. Remove code for propagating phase from existing commands (e.g. amend)
  
  Perhaps I'll find some time to work on this during the sprint, but it does seem a little big to be completed during the sprint.
  
  [1] It seems like we should do the same with bookmarks -- move them as we go, but move them back on abort. However, I'm not sure if that's BC. Can we pretend that it was a bug that they didn't? Maybe we don't care much about BC of interrupted rebase/histedit?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2013

To: spectral, #hg-reviewers
Cc: martinvonz, mercurial-devel