Summary:
As suggested by wez, "dim" is less supported than the bright colors.
So let's use the bright versions instead. Basically, replacing "red",
"red dim" with "brightred" and "red".
Reviewed By: singhsrb
Differential Revision: D7387254
fbshipit-source-id: f741d9e1c12115acdd1a3a2cae1658d8fae534bf
Summary:
worddiff needs extra computation and does not have effect if color is
disabled. It should also be disabled with HGPLAIN=1.
Reviewed By: ryanmce
Differential Revision: D7315276
fbshipit-source-id: b59255bc708713186bd4ddafdc469763e1a66ea6
Summary:
There were recent complains about both quality [1] [2] and performance [3]
of the current word diff algorithm.
The current algorithm is actually bad in various ways:
- Lines could be matched across hunks, which is confusing (report [1]).
- For short lines, they can fail "similarity" check, which means they
won't be highlighted when they are expected to be (report [2]).
- Various performance issues:
- Using difflib implemented by pure Python, which is both slow and
suboptimal comparing with xdiff.
- Searching for matched lines across hunks could be O(N^2) if there are
no match found.
Thinking it in a "highlight" way is actually tricky, consider the following
change:
```
# before
foo = 10
# after
if True:
foo = 21 + 3
```
It's obvious that "10" and "21 + 3" need highlighting because they are
different. But what about "if True:"? In theory it's also "different" and
need highlighting. How about purely inserted or deleted hunks then?
Highlighting all of them would be too noisy.
This diff rewrites the word diff algorithm. It differs in multiple ways:
1. Get rid of "matching lines by similarity" step.
2. Only diff words within a same hunk.
3. Dim unchanged words. Instead of highlighting changed words.
4. Treat pure insertion or deletion hunks differently - do not dim or
highlight words in them.
5. Use xdiff instead.
6. Use a better regexp to split words. This reduces the number of tokens sent
to the diff algorithm.
1, 2, 5, 6 help performance. 1, 2, 3, 4 make the result more predictable and
trustworthy. 3 avoids the nasty question about what to highlight. 3 and 4 makes
it more flexible for people to tweak colors. 6 makes the result better since it
merges multiple space tokens into one so xdiff will less likely miss important
matches (than meaningless matches like spaces).
"bold" and "underline" were removed so the changed words will have regular
red/green colors. The output won't be too "noisy" even in cases where code are
changed in a way that inline word matching is meaningless. For people who want
more contrast, they can set:
[color]
diff.inserted.changed = green bold
diff.deleted.changed = red bold
Practically, when diffing D7319718, the old code spends 4 seconds on finding
matched lines preparing for worddiff:
```
| diffordiffstat cmdutil.py:1522
\ difflabel (17467 times) patch.py:2471
....
> 3927 \ _findmatches (22 times) patch.py:2537
348 \ __init__ (8158 times) difflib.py:154
340 | set_seqs (8158 times) difflib.py:223
328 | set_seq2 (8158 times) difflib.py:261
322 | __chain_b (8158 times) difflib.py:306
1818 \ ratio (8158 times) difflib.py:636
1777 | get_matching_blocks (8158 times) difflib.py:460
1605 \ find_longest_match (51966 times) difflib.py:350
38 | __new__ (51966 times) <string>:8
29 \ _make (36035 times) <string>:12
143 \ write (17466 times) ui.py:883
```
The new code takes 0.14 seconds:
```
| diffordiffstat cmdutil.py:1522
\ difflabel (23401 times) patch.py:2562
....
> 140 \ consumehunkbuffer (23346 times) patch.py:2585
130 | diffsinglehunkinline (23240 times) patch.py:2496
215 \ write (23400 times) ui.py:883
118 \ flush cmdutil.py:1606
118 | write ui.py:883
```
[1]: https://fburl.com/lkb9rc9m
[2]: https://fburl.com/0r9bqf0e
[3]: https://fburl.com/pxqznw31
Reviewed By: ryanmce
Differential Revision: D7314726
fbshipit-source-id: becd979cb9ac3fd3f4adae11cb10804d535f58df
Summary:
Instead of yielding tokens directly, buffer them if they belong to a same
hunk. This makes it easier for the upcoming new worddiff algorithm to only
focus on the diff hunk, instead of having to worry about other contents.
This breaks how the existing experimental worddiff algorithm works, so the
algorithm was removed, and related tests are disabled for now.
Reviewed By: ryanmce
Differential Revision: D7314725
fbshipit-source-id: 344e502cd185b2412fbd2ee299131bbb4560ac17
This changes {\W} to {\W - any 8bit characters} so that multibyte sequences
are taken as words. Since we don't know the encoding of user content, this
is the most sensible definition of a non-word.
The `diff' command usually writes deletion in red and insertions in green. This
patch adds within-line colors, to highlight which part of the lines differ.
Lines to compare are decided based on their similarity ratio, as computed by
difflib SequenceMatcher, with an arbitrary threshold (0.7) to decide at which
point two lines are considered entirely different (therefore no inline-diff
required).
The current implementation is kept behind an experimental flag in order to test
the effect on performance. In order to activate it, set inline-color-diff to
true in [experimental].
Windows uses `more.com`, which unhelpfully adds an extra trailing line
consisting only of '\r'. It also converts tab characters to spaces, which
throws off the last two tests.
Setting the 'ui.formatted' option is what allowed the pager to be used by these
tests in the first place.
Previously, 'ui.color=yes' meant "always show color", While
"ui.color=auto" meant "use color automatically when it appears
sensible".
This feels problematic to some people because if an administrator has
disabled color with "ui.color=off", and a user turn it back on using
"color=on", it will get surprised (because it breaks their output when
redirected to a file.) This patch changes ui.color=true to only move the
default value of --color from "never" to "auto".
I'm not really in favor of this changes as I suspect the above case will
be pretty rare and I would rather keep the logic simpler. However, I'm
providing this patch to help the 4.2 release in the case were others
decide to make this changes.
Users that want to force colors without specifying --color on the
command line can use the 'ui.formatted' config knob, which had to be
enabled in a handful of tests for this patch.
Nice summary table (credit: Augie Fackler)
That is, before this patch:
+--------------------+--------------------+--------------------+
| | not a tty | a tty |
| | --color not set | --color not set |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color (not set) | no color | no color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = auto | no color | color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = yes | *color* | color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = no | no color | no color |
| | | |
+--------------------+--------------------+--------------------+
(if --color is specified, it always clobbers the setting in [ui])
and after this patch:
+--------------------+--------------------+--------------------+
| | not a tty | a tty |
| | --color not set | --color not set |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color (not set) | no color | no color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = auto | no color | color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = yes | *no color* | color |
| | | |
+--------------------+--------------------+--------------------+
| [ui] | | |
| color = no | no color | no color |
| | | |
+--------------------+--------------------+--------------------+
(if --color is specified, it always clobbers the setting in [ui])
This new option control whether or not color will be used. It mirror the behavior
of '--color'. I usually avoid adding new option to '[ui]' as the section is
already filled with many option. However, I feel like 'color' is central enough
to deserves a spot in this '[ui]' section.
For now the option is not documented so it is still marked as experimental. Once
it get documented and official, we should be able to deprecate the color
extensions.
There is more cleanup to do before that documentation is written, but we need
this option early to made them. Having that option will allow for more cleanup
of the initialisation process and proper separation between color
configuration.
The tests often set ui.interactive to control normally interactive prompts from
stdin. That gave an output where it was non-obvious what prompts got which
which response, and the output lacked the newline users would see after input.
Instead, if the input not is a tty, write the selection and a newline.
The following patch splits up changed lines along tabs (using
re.findall), and gives them a "diff.tab" label. This can be used by
the color extension for colorising tabs, like it does right now with
trailing whitespace.
I also provide corresponding tests.
Color extension achieves colorization by overriding the class of
"ui" object just before command execution.
Before this patch, "diff()" of abstractsubrepo and classes
derived from it has no "ui" argument, so "diff()" of hgsubrepo
uses "self._repo.ui" to invoke "cmdutil.diffordiffstat()".
For separation of configuration between repositories, revision
1498948ee815 changed the initialization source of "self._repo.ui"
from "ui"(overridden) to "baseui"(plain) of parent repository.
And this caused break of colorization.
This patch adds "ui" argument to "diff()" of abstractsubrepo and
classes derived from it to pass "ui" object of caller side.
Many tests didn't change back from subdirectories at the end of the tests ...
and they don't have to. The missing 'cd ..' could always be added when another
test case is added to the test file.
This change do that tests (99.5%) consistently end up in $TESTDIR where they
started, thus making it simpler to extend them or move them around.
It is possible that unrelated changes in a file are on sequential lines. The
current record extension does not allow these to be committed independently.
An example use case for this is in software development for deeply embedded
real-time systems. In these environments, it is not always possible to use a
debugger (due to time-constraints) and hence inline UART-based printing is
often used. When fixing a bug in a module, it is often convenient to add a
large number of 'printf's (linked to the UART via a custom fputc) to the module
in order to work out what is going wrong. printf is a very slow function (and
also variadic so somewhat frowned upon by the MISRA standard) and hence it is
highly undesirable to commit these lines to the repository. If only a partial
fix is implemented, however, it is desirable to commit the fix without deleting
all of the printf lines. This is also simplifies removal of the printf lines
as once the final fix is committed, 'hg revert' does the rest. It is likely
that the printf lines will be very near the actual fix, so being able to split
the hunk is very useful in this case.
There were two alternatives I considered for the user interface. One was to
manually edit the patch, the other to allow a hunk to be split into individual
lines for consideration. The latter option would require a significant
refactor of the record module and is less flexible. While the former is
potentially more complicated to use, this is a feature that is likely to only
be used in certain exceptional cases (such as the use case proposed above) and
hence I felt that the complexity would not be a considerable issue.
I've also written a follow-up patch that refactors the 'prompt' code to base
everything on the choices variable. This tidies up and clarifies the code a
bit (removes constructs like 'if ret == 7' and removes the 'e' option from the
file scope options as it's not relevant there. It's not really a necessity, so
I've excluded it from this submission for now, but I can send it separately if
there's a desire and it's on bitbucket (see below) in the meantime.
Possible future improvements include:
* Tidying up the 'prompt' code to base everything on the choices variable.
This would allow entries to be removed from the prompt as currently 'e' is
offered even for entire file patches, which is currently unsupported.
* Allowing the entire file (or even multi-file) patch to be edited manually:
this would require quite a large refactor without much benefit, so I decided
to exclude it from the initial submission.
* Allow the option to retry if a patch fails to apply (this is what Git does).
This would require quite a bit of refactoring given the current 'hg record'
implementation, so it's debatable whether it's worth it.
Output is similar to existing record user interface except that an additional
option ('e') exists to allow manual editing of the patch. This opens the
user's configured editor with the patch. A comment is added to the bottom of
the patch explaining what to do (based on Git's one).
A large proportion of the changeset is test-case changes to update the options
reported by record (Ynesfdaq? instead of Ynsfdaq?). Functional changes are in
record.py and there are some new test cases in test-record.t.
Using terminfo instead of hard-coding ECMA-48 control sequences provides a
greater assurance that the terminal codes are correct for the current
terminal type; not everything supports the ANSI escape codes.
It also allows us to use a wider range of colors when a terminal emulator
supports it (such as 16- or 256-color xterm), and a few more non-color
attributes, such as the ever-popular blink.
Previously, when rolling back a transaction, some users could be confused
between the level to which the store is rolled back, and the new parents
of the working directory.
$ hg rollback
rolling back to revision 4 (undo commit)
With this change:
$ hg rollback
repository tip rolled back to tip revision 4 (undo commit)
working directory now based on revision 2 and 1
So now the user can realize that the store has been rolled back to an older
tip, but also that the working directory may not on the tip (here we are
rolling back the merge of the heads 2 and 1)
