( )⚙ D6562 filemerge: make last line of prompts <40 english chars (issue6158)

This is an archive of the discontinued Mercurial Phabricator instance.

filemerge: make last line of prompts <40 english chars (issue6158)
ClosedPublic

Authored by spectral on Jun 20 2019, 6:09 PM.

Details

Summary

I've chosen <40 as the target so that other languages that may have a 2x blowup
in character count can still have a chance to fit into an 80 column screen.

Previously, we would show a prompt like:

keep (l)ocal [dest], take (o)ther [source], or leave (u)nresolved for some/potentially/really/long/path?

On at least some systems, if readline was in use then the last line of the
prompt would be wrapped strangely if it couldn't fit entirely on one line. This
strange wrapping may be just a carriage return without a line feed, overwriting
the beginning of the line; example (100 columns wide, 65 character filename, and
yes there's 10 spaces on the end, I assume this is to handle the user inputting
longest word we provide as an option, "unresolved"):

ng/dir/name/that/does/not/work/well/with/readline/file.txt? ave (u)nresolved for some/lon

In some cases it may partially wrap onto the next line, but still be missing
earlier parts in the line, such as below (60 columns wide, 65 character
filename):

 rev], or leave (u)nresolved for some/long/dir/name/that/do
s/not/work/well/with/readline/file.txt?

With this fix, this looks like this on a 60 column screen:

tool vim_with_markers (for pattern some/long/dir/name/that/d
oes/not/work/well/with/readline/file.txt) can't handle binar
y
tool meld can't handle binary
tool vim_with_markers can't handle binary
tool internal:merge3 can't handle binary
tool merge can't handle binary
no tool found to merge some/long/dir/name/that/does/not/work
/well/with/readline/file.txt
file 'some/long/dir/name/that/does/not/work/well/with/readli
ne/file.txt' needs to be resolved.
You can keep (l)ocal [working copy], take (o)ther [merge rev
], or leave (u)nresolved.
What do you want to do?

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

spectral created this revision.Jun 20 2019, 6:09 PM

I like this, but could I get you to add a patch 3 that introduces a develwarn for this?

I like this, but could I get you to add a patch 3 that introduces a develwarn for this?

Done, and it found a couple other cases, but I'm not sure I like it (D6573) - non-English users might not be able to enable the devel warnings without getting a bunch of spam. I believe by the time we get to ui.prompt() we've already done the _() translation and it's in the target language? Do we have any idea if anyone enables developer warnings outside of tests (which would typically be written in English, at least for core).

Unfortunately, I don't think it's something we could put in check-style.py either... we won't know that these are going to a prompt. Anyway, we can continue discussion on this on D6573.

Ah, a question: should this be lowercasing the first character of things? I think I copied this from somewhere where it did have an uppercase letter, but I wasn't consistent in my application of the leading uppercase letter.

I like this, but could I get you to add a patch 3 that introduces a develwarn for this?

Done, and it found a couple other cases, but I'm not sure I like it (D6573) - non-English users might not be able to enable the devel warnings without getting a bunch of spam. I believe by the time we get to ui.prompt() we've already done the _() translation and it's in the target language? Do we have any idea if anyone enables developer warnings outside of tests (which would typically be written in English, at least for core).
Unfortunately, I don't think it's something we could put in check-style.py either... we won't know that these are going to a prompt. Anyway, we can continue discussion on this on D6573.

Oh. Yeah, good point.

Welp. I guess the best bet is to not land that last patch then. :(

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.