This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9853

histedit: don't assign to _ for unused values
ClosedPublic

Authored by durin42 on Jan 22 2021, 3:46 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG8477c91b5e8e: histedit: don't assign to _ for unused values
rHGfe14325ee0a2: histedit: don't assign to _ for unused values
Summary

I don't know what this ignored value is, but we need to not clobber
the _() function from gettext, or we get mysterious crashes instead of
internationalizing some strings in my upcoming patches.

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

durin42 created this revision.Jan 22 2021, 3:46 PM
SimonSapin added a subscriber: SimonSapin.Jan 26 2021, 1:37 AM

I half-expected this to fail some linter, but I see tests/test-check-pylint.t only enables very few checks. Still, it might be good to call unused variables _unused with the _ prefix for intentionally unused?

The repo has some other uses of _ in deconstructing tuples that might similarly conflict with gettext. I wouldn’t try to go after them all without an automated check but this file has 4 other uses of mode, _ = state[b'mode']

pulkit accepted this revision.Jan 26 2021, 3:03 PM
This revision is now accepted and ready to land.Jan 26 2021, 3:03 PM
durin42 added a commit: rHGfe14325ee0a2: histedit: don't assign to _ for unused values.Jan 27 2021, 8:13 AM
Closed by commit rHGfe14325ee0a2: histedit: don't assign to _ for unused values (authored by durin42). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/histedit.py (4 lines)

Diff 25296

hgext/histedit.py

} }
# eventloop # eventloop
ch = None ch = None
stdscr.clear() stdscr.clear()
stdscr.refresh() stdscr.refresh()
while True: while True:
try: try:
oldmode, _ = state[b'mode'] oldmode, unused = state[b'mode']
if oldmode == MODE_INIT: if oldmode == MODE_INIT:
changemode(state, MODE_RULES) changemode(state, MODE_RULES)
e = event(state, ch) e = event(state, ch)
if e == E_QUIT: if e == E_QUIT:
return False return False
if e == E_HISTEDIT: if e == E_HISTEDIT:
return state[b'rules'] return state[b'rules']
else: else:
if e == E_RESIZE: if e == E_RESIZE:
size = screen_size() size = screen_size()
if size != stdscr.getmaxyx(): if size != stdscr.getmaxyx():
curses.resizeterm(*size) curses.resizeterm(*size)
curmode, _ = state[b'mode'] curmode, unused = state[b'mode']
sizes = layout(curmode) sizes = layout(curmode)
if curmode != oldmode: if curmode != oldmode:
state[b'page_height'] = sizes[b'main'][0] state[b'page_height'] = sizes[b'main'][0]
# Adjust the view to fit the current screen size. # Adjust the view to fit the current screen size.
movecursor(state, state[b'pos'], state[b'pos']) movecursor(state, state[b'pos'], state[b'pos'])
# Pack the windows against the top, each pane spread across the # Pack the windows against the top, each pane spread across the
# full width of the screen. # full width of the screen.