Summary: Mostly empty lines removed and added. A few bugfixes on excessive line splitting.
Reviewed By: quark-zju
Differential Revision: D8199128
fbshipit-source-id: 90c1616061bfd7cfbba0b75f03f89683340374d5
Summary:
Turned on the auto formatter. Ran `arc lint --apply-patches --take BLACK **/*.py`.
Then run `arc lint` again so some other autofixers like spellchecker etc. looked
at the code base. Manually accept the changes whenever they make sense, or use
a workaround (ex. changing "dict()" to "dict constructor") where autofix is false
positive. Disabled linters on files that are hard (i18n/polib.py) to fix, or less
interesting to fix (hgsubversion tests), or cannot be fixed without breaking
OSS build (FBPYTHON4).
Conflicted linters (test-check-module-imports.t, part of test-check-code.t,
test-check-pyflakes.t) are removed or disabled.
Duplicated linters (test-check-pyflakes.t, test-check-pylint.t) are removed.
An issue of the auto-formatter is lines are no longer guarnateed to be <= 80
chars. But that seems less important comparing with the benefit auto-formatter
provides.
As we're here, also remove test-check-py3-compat.t, as it is currently broken
if `PYTHON3=/bin/python3` is set.
Reviewed By: wez, phillco, simpkins, pkaush, singhsrb
Differential Revision: D8173629
fbshipit-source-id: 90e248ae0c5e6eaadbe25520a6ee42d32005621b
Summary:
Nothing currently displays a progress bar while the curses interface is shown,
but in the future something may be added which does, so ensure the progress bar
is suspended when showing the curses interface.
Reviewed By: ryanmce
Differential Revision: D7417435
fbshipit-source-id: 6b91b17ee5390cbde6e983081a0940051ab865c8
Summary:
crecord locked out the amend functionality to older versions of
Mercurial. Since it's packaged in core now, let's always enable it.
Reviewed By: quark-zju
Differential Revision: D6837724
fbshipit-source-id: 2a0dae2e03346734f7d8298e5a073f776849005e
3649c3f2cd (revert: do not reverse hunks in interactive when REV is not
parent (issue5096)) changed the record "operation" for the text version but
missed the curses version. Without this patch, running
`hg revert -ir '.^' --config ui.interface=curses` would crash with:
ProgrammingError: unexpected operation: apply
Differential Revision: https://phab.mercurial-scm.org/D1381
Changes the API of `ui.edit()` to take an optional `action` argument,
which is used when constructing the suffix of the temp file.
Previously, it was possible to set the suffix by specifying a `suffix` to the
optional `extra` dict that was passed to `ui.edit()`, but the goal is to
drop support for `extra.suffix` and make `action` a required argument.
To this end, `ui.edit()` now yields a `develwarn()` if `action` is not set
or if `extra.suffix` is set.
I updated all calls to `ui.edit()` I could find in `hg-crew` to specify the
appropriate `action`. This means that when creating a commit, instead
of the path to the editor file being something like:
`/tmp/hg-editor-XXXXXX.txt`
it is now something like:
`/tmp/hg-editor-XXXXXX.commit.hg.txt`
Some editors (such as Atom) make it possible to statically define a [TextMate]
grammar for files with a particular suffix. For example, because Git reliably
uses `.git/COMMIT_EDITMSG` and `.git/MERGE_MSG` as the paths for commit-type
messages, it is trivial to define a grammar that is applied when files of
either name are opened in Atom:
https://github.com/atom/language-git/blob/v0.19.1/grammars/git%20commit%20message.cson#L4-L5
Because Hg historically used a generic `.txt` suffix, it was much harder to
disambiguate whether a file was an arbitrary text file as opposed to one
created for the specific purpose of authoring an Hg commit message.
This also makes it easier to add special support for `histedit`, as it has its own
suffix that is distinct from a commit:
`/tmp/hg-histedit-XXXXXX.histedit.hg.txt`
Test Plan:
Added an integration test: `test-editor-filename.t`.
Manually tested: ran `hg ci --amend` for this change and saw that it
used `/tmp/hg-editor-ZZjcz0.commit.hg.txt` as the path instead of
`/tmp/hg-editor-ZZjcz0.txt` as the path.
Verified `make tests` passes.
Differential Revision: https://phab.mercurial-scm.org/D464
With the former crecord extension, the user could edit the commit
message while he was de-/selecting hunks. By pressing 'm', an editor
showed up to edit the commit message.
With record being part of mercurial, this feature is not available
anymore. However, the help text still mentions it.
As the infrastructure needed is still present, this feature is quite
easily ported from the crecord extension to mercurial.
It seems there is no test coverage for record ui, so I tested this patch
manually on my local machine.
The status line in the crecord has the "space" status field which has variable
length depending on the length of the status label in the language of choice.
In English, the status labels are "space: deselect" and "space:select". The
"deselect" label is 2 glyphs longer. This makes the terminal output jump
around if the terminal width is just right so that the shorter label makes
the status line 1 line long, and the longer label makes it 2 lines long.
This patch formats the selected status into a fixed-width field. The field
width is the maximum of the lengths of the two possible labels, to account for
differing translations and label lengths. This should make the label behavior
uniform across localizations.
There does not seem to be a test for crecord, so I verified the change manually
with a local build of 'hg'.
The old reversehunks code accesses "crecord.uihunk._hunk", which is the raw
recordhunk without crecord selection information, therefore "revert -i"
cannot revert individual lines, aka. issue5337.
The patch rewrites related logic to return the right reverse hunk for
revert. Namely,
1. "fromline" and "toline" are correctly swapped [1]
2. crecord.uihunk generates a correct reverse hunk [2]
Besides, reversehunks(hunks) will no longer modify its input "hunks", which
is more expected.
[1]: To explain why "fromline" and "toline" need to be swapped, take the
following example:
$ cat > a <<EOF
> 1
> 2
> 3
> 4
> EOF
$ cat > b <<EOF
> 2
> 3
> 5
> EOF
$ diff a b
1d0 <---- "1" is "fromline" and "0" is "toline"
< 1 and they are swapped if diff from the reversed direction
4c3 |
< 4 |
--- |
> 5 |
|
$ diff b a |
0a1 <---------+
> 1
3c4 <---- also "4c3" gets swapped to "3c4"
< 5
---
> 4
[2]: This is a bit tricky.
For example, given a file which is empty in working parent but has 3 lines
in working copy, and the user selection:
select hunk to discard
[x] +1
[ ] +2
[x] +3
The user intent is to drop "1" and "3" in working copy but keep "2", so the
reverse patch would be something like:
-1
2 (2 is a "context line")
-3
We cannot just take all selected lines and swap "-" and "+", which will be:
-1
-3
That patch won't apply because of "2". So the correct way is to insert "2"
as a "context line" by inserting it first then deleting it:
-2
+2
Therefore, the correct revert patch is:
-1
-2
+2
-3
It could be reordered to look more like a common diff hunk:
-1
-2
-3
+2
Note: It's possible to return multiple hunks so there won't be lines like
"-2", "+2". But the current implementation is much simpler.
For deletions, like the working parent has "1\n2\n3\n" and it was changed to
empty in working copy:
select hunk to discard
[x] -1
[ ] -2
[x] -3
The user intent is to drop the deletion of 1 and 3 (in other words, keep
those lines), but still delete "2".
The reverse patch is meant to be applied to working copy which is empty.
So the patch would be:
+1
+3
That is to say, there is no need to special handle the unselected "2" like
the above insertion case.
There are some setup and cleanup necessary around the main code, that
setup/cleanup code needs multiple adjustments so we extract the core code into
its own function first for clarity.
I really want to have an option of toggling a selection on a line and also
moving cursor down as a single keystroke. It also kinda makes sense for space
key to do this, because some other curses UIs in the wild do this (e.g. various
file managers, htop). So I got an idea to make a config option that defaults to
False for compatibility, but allows making crecord UI a lot more useful for
people with big hunks.
We add this an experimental option to experiment with this behavior.
This will make crecord consistent with record when being used in the revert
situation. It will say "Select hunks to revert / discard" accordingly.
This should make the revert crecord interface less confusing.
Previously, we display fixed text in the 2-line status header. Now we want
to customize some word there to make the "revert" action clear. However, if
we simply replace the verb using '%s' like this:
"SELECT CHUNKS: (j/k/up/dn/pgup/pgdn) move cursor; "
"(space/A) toggle hunk/all; (e)dit hunk;"),
" (f)old/unfold; (c)onfirm %s; (q)uit; (?) help " % verb
"| [X]=hunk %s **=folded, toggle [a]mend mode" % verb
It could cause trouble for i18n - some languages may expect things like
"%(verb) confirm", for example.
Therefore, this patch chooses to break the hard-coded 2-line sentences into
"segment"s which could be translated (and replaced) separately.
With the clean-up, I'm also changing the content being displayed, to make it
cleaner and more friendly to (new) users, namely:
- Replace "SELECT CHUNKS" to "Select hunks to record". Because:
- To eliminate "hunk" / "chunk" inconsistency.
- "record" is used in the "text" UI. Do not use "apply", to make it
consistent.
- To make it clear you are choosing what to record, not revert, or
discard etc. This is probably the most important information the user
should know. So let's put it first.
- "to record" could be replaced to others depending on the operation.
The follow-up patches will address them.
- Move "[x]" and "**" legends first to explain the current interface. New
users should understand what the legends mean, followed by what they can
do in the interface.
- Replace "j/k/up/dn/pgup/pgdn" with "arrow keys". Because:
- "arrow keys" is more friendly to new users.
- Mentioning "j/k" first may make emacs users angry. We should stay
neutral about editors.
- "pgup/pgdn" actually don't work very well. For example, within a hunk
of 100-line insertion, "pgdn" just moves one single line.
- Left/Right arrow keys are useful for movement and discovery of
"expanding" a block.
- Replace "fold/unfold" with "collapse/expand", "fold" is well known as
a different meaning in histedit and evolve.
- Replace "(space/A) toggle hunk/all" to "space: select". Because:
- "A: toggle all" is not that useful
- It's unclear how "hunk" could be "toggled" to a dumb user. Let's
make it clear it's all about "select".
- A follow-up will make it use "unselect" when we know the current item
is selected.
- Remove "(f)old". Use arrow keys instead.
- Remove "toggle [a]mend mode". It's just confusing and could be useless.
- Remove "(e)dit hunk". It's powerful but not friendly to new users.
- Replace "(q)uit" to "q: abort" to make it clear you will lose changes.
The result looks like the following in a 73-char-width terminal:
Select hunks to record - [x]=selected **=collapsed c: confirm q: abort
arrow keys: move/expand/collapse space: select ?: help
If the terminal is 132-char wide, the text could fit in a single line.
We are going to make the text in the status window dynamically generated,
so its size would be dynamic. Change getstatuslines to update
"numstatuslines" automatically. Fix an issue where "numstatuslines" being 1
makes the chunkpad disappear.
We will do some changes there in the next patches. The new method would also
be the "source of truth" of the content of the status window (so if the
status window needs more than 2 lines, it would be calculated from the new
method).
Also, moved "statuswin.refresh" to make the code compact and easier to read.
Using ctrl-l for this purpose seems to be a fairly widely used practice,
presumably following emacs. This doesn't scroll the selected line all
the way to the top of the window, instead it leaves a 3 line buffer for
context. Use curses.unctrl() to resolve keypressed to '^L' to avoid
hard-coding hexadecimal key codes.
The 3 classes for items used in crecord (uiheader, uihunk, uihunkline) all have
prevsibling() and nextsibling() methods. The two methods are used to get the
previous/next item of the same type of the same parent element as the current
one: when `a` is a uihunkline instance, a.nextsibling() returns the next line
in this hunk (or None, if `a` is the last line).
There are also two similar methods: previtem() and nextitem(). When called with
constrainlevel=True (the default) they simply returned the result of
prevsibling()/nextsibling(). Only when called with constrainlevel=False they
did something different: they returned previous/next item regardless of its
type (so if `a` is the last line in a hunk, a.nextitem(constrainlevel=False)
could return the next hunk or the next file -- something that is not a line).
Let's simplify this logic and make code call -sibling() methods when only
siblings are needed and -item() methods when any item would do, and then remove
the constrainlevel argument from previtem() and nextitem().
Scrolling screen is currently done in a different place. The things that had
been described in the docstrings may still happen, but the functions touched by
this patch don't do any scrolling, they only set self.currentselecteditem and
nothing more.
Previously, the SIGWINCH handler does not get cleared and if the commit
message editor also needs SIGWINCH handling (like vim), the two SIGWINCH
handlers (the editor's, ours) will have a race. And we may erase the
editor's screen content.
This patch restores SIGWINCH handler to address the above issue.
The commit confirmation is not very useful -- it gives no way to view what you
have selected, so you're blindly choosing whether to proceed or not, and it adds
a lot of unnecessary friction to committing. In addition, we now have a working
'review' choice for those who really want to review the final change.
Ryan McElroy initially submitted a config option to make this optional, but we
never saw a V2. However as the freeze is near and curses have never been
officially out of the door, I think it is worth skipping the config and trying
getting it right for this release.
The "r" option for this feature was copied into Mercurial from
crecord, but the actual implementation never made it into hg until
now. It's a moderately useful feature that allows the user to edit the
patch in a text editor before comitting it for good.
This requires a test, so we must also enable a corresponding testing
'R' option that skips the confirmation dialogue. In addition, we also
need a help text for the editor when reviewing the final patch.
As for why this is a useful feature if we can already edit hunks in an
editor, I would like to offer the following points:
* editing hunks does not show the entire patch all at once
** furthermore, the hunk "tree" in the TUI has no root that could be
selected for edition
* it is helpful to be able to see the entire final patch for
confirmation
** within this view, the unselected hunks are hidden, which is
visusally cleaner
** this works as a final review of the complete result, which is
a bit more difficult to do conceptually via hunk editing
* this feature was already in crecord, so it was an oversight to
not bring it to core
* it works and is consistent with editing hunks
This help message can be useful for other situations, such as for the
review extension. It's also easier to write it at the top-level
indentation with triple-quoted strings instead of inserting comment
characters and newlines programmatically.
