[PATCH 1 of 5 shelve-ext] shelve: move temporary commit creation to a separate function

[Yuya Nishihara]

On Tue, 3 Jan 2017 12:13:35 +0100, Pierre-Yves David wrote:
> On 11/20/2016 05:10 AM, Yuya Nishihara wrote:
> > On Sun, 20 Nov 2016 00:42:53 +0000, Kostia Balytskyi wrote:
> >> On 11/19/16, 10:02 AM, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:
> >>     > +def _commitworkingcopychanges(ui, repo, opts, tmpwctx):
> >>     > +    """Temporarily commit working copy changes before moving unshelve commit"""
> >>     > +    # Store pending changes in a commit and remember added in case a shelve
> >>     > +    # contains unknown files that are part of the pending change
> >>     > +    s = repo.status()
> >>     > +    addedbefore = frozenset(s.added)
> >>     > +    if not (s.modified or s.added or s.removed or s.deleted):
> >>     > +        return tmpwctx, addedbefore
> >>     > +    ui.status(_("temporarily committing pending changes "
> >>     > +                "(restore with 'hg unshelve --abort')\n"))
> >>     > +    commitfunc = getcommitfunc(extra=None, interactive=False,
> >>     > +                               editor=False)
> >>     > +    tempopts = {}
> >>     > +    tempopts['message'] = "pending changes temporary commit"
> >>     > +    tempopts['date'] = opts.get('date')
> >>     > +    ui.quiet = True
> >>     > +    node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)
> >>     > +    tmpwctx = repo[node]
> >>     > +    return tmpwctx, addedbefore
> >>
> >>     This and the next unbalanced ui.quiet hack can be a source of future bugs.
> >>     I hope they'll be fixed by a follow-up patch.
> >
> >> Can you explain please? I was trying to keep the ui.quiet behavior unchanged in this refactoring series.
> >
> > You've split set/restore of ui.quiet to separate functions. It's unclear that
> > we have to care about the state of ui.quiet when adding some code to
> > _dounshelve(). So IMO, each function should restore ui.quiet at the end.
> 
> I don't think this followup ever happen (but having a quick look at the 
> code. What is the status of this?

Not yet, but perhaps b0a8337ba9af "ui: add configoverride context manager"
was the patch to start addressing this.