This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5489

histedit: add warning message on editing tagged commits (issue4017)
ClosedPublic

Authored by navaneeth.suresh on Jan 3 2019, 1:33 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG86f0ed7ac688: histedit: add warning message on editing tagged commits (issue4017)

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Closed by commit rHG86f0ed7ac688: histedit: add warning message on editing tagged commits (issue4017) (authored by navaneeth.suresh). · Explain WhyJan 3 2019, 5:53 PM
This revision was automatically updated to reflect the committed changes.
navaneeth.suresh added a comment.Jan 3 2019, 10:50 PM

Thanks for queuing!

mharbison72 added a subscriber: mharbison72.Jan 3 2019, 11:02 PM
mharbison72 added inline comments.
hgext/histedit.py
1707

Out of curiosity, why the sleep?

navaneeth.suresh added inline comments.Jan 4 2019, 3:28 AM
hgext/histedit.py
1707

Otherwise, the user won't be able to see the warning as the editor opens for histedit.

mharbison72 added inline comments.Jan 5 2019, 9:09 PM
hgext/histedit.py
1707

Oh, OK. Does it make sense to make this an interactive prompt to continue/bail instead, to ensure this isn't missed? (Don't feel obligated- I wasn't going to say anything because I couldn't think of any other commands that prompt like this. And then I remembered that absorb does.)

yuja added a subscriber: yuja.Jan 5 2019, 9:14 PM

mharbison72 wrote in histedit.py:1707
Out of curiosity, why the sleep?

Otherwise, the user won't be able to see the warning as the editor opens for histedit.

Maybe ui.prompt()?

I don't like this sort of sleep() use. 1 sec wouldn't be enough to notice the
warning.

navaneeth.suresh added a comment.Jan 5 2019, 11:15 PM

@mharbison72 @yuja Thank you for the suggestions. I've created a follow-up patch making the suggested changes. Please see D5494.

Revision Contents
Changeset List

PathPackages
Mhgext/histedit.py (13 lines)

Diff 13000

hgext/histedit.py

import termios import termios
except ImportError: except ImportError:
fcntl = None fcntl = None
termios = None termios = None
import functools import functools
import os import os
import struct import struct
import time
from mercurial.i18n import _ from mercurial.i18n import _
from mercurial import ( from mercurial import (
bundle2, bundle2,
cmdutil, cmdutil,
context, context,
copies, copies,
destutil, destutil,
revs = opts.get('rev', []) revs = opts.get('rev', [])
# experimental config: ui.history-editing-backup # experimental config: ui.history-editing-backup
nobackup = not ui.configbool('ui', 'history-editing-backup') nobackup = not ui.configbool('ui', 'history-editing-backup')
rules = opts.get('commands', '') rules = opts.get('commands', '')
state.keep = opts.get('keep', False) state.keep = opts.get('keep', False)
_validateargs(ui, repo, state, freeargs, opts, goal, rules, revs) _validateargs(ui, repo, state, freeargs, opts, goal, rules, revs)
hastags = False
if revs:
revs = scmutil.revrange(repo, revs)
ctxs = [repo[rev] for rev in revs]
for ctx in ctxs:
tags = [tag for tag in ctx.tags() if tag != 'tip']
if not hastags:
hastags = len(tags)
if hastags:
ui.warn(_('warning: tags associated with the given changeset '
'will be lost after histedit \n'))
time.sleep(1)
mharbison72Unsubmitted
Not Done

Out of curiosity, why the sleep?

mharbison72: Out of curiosity, why the sleep?
navaneeth.sureshAuthorUnsubmitted
Not Done

Otherwise, the user won't be able to see the warning as the editor opens for histedit.

navaneeth.suresh: Otherwise, the user won't be able to see the warning as the editor opens for histedit.
mharbison72Unsubmitted
Not Done

Oh, OK. Does it make sense to make this an interactive prompt to continue/bail instead, to ensure this isn't missed? (Don't feel obligated- I wasn't going to say anything because I couldn't think of any other commands that prompt like this. And then I remembered that absorb does.)

mharbison72: Oh, OK. Does it make sense to make this an interactive prompt to continue/bail instead, to…
# rebuild state # rebuild state
if goal == goalcontinue: if goal == goalcontinue:
state.read() state.read()
state = bootstrapcontinue(ui, state, opts) state = bootstrapcontinue(ui, state, opts)
elif goal == goaleditplan: elif goal == goaleditplan:
_edithisteditplan(ui, repo, state, rules) _edithisteditplan(ui, repo, state, rules)
return return
elif goal == goalabort: elif goal == goalabort: