This should prevent long lines from making the code width different between files, which can be annoying. (And of course, it stops long lines from making a giant scrollbar too.)
Details
Details
- Reviewers
epriestley chad - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rP9fedd343eb18: Break long words in differential two-up view
Loaded this diff in Chrome, Firefox, IE9, and IE8:
(That's a screenshot from Chrome, but it looks about the same in the other browsers.)
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Switch to unicode 'word joiner' (essentially a non-breaking zero-width space) so there's no extra line break when a line starts a really long word. Now looks like:
{F137518}
Note that the line number in beta.txt lines up with the line. (Tested copying in Chrome, Firefox, IE9. IE8 already didn't work.)
Comment Actions
About the same as before, but with line breaking. Here's an example that has some red.
When you scroll over, it looks reasonable too:
Comment Actions
i'd defer to @epriestley, not sure how purposeful it is to not break words - it would be my guess someone will dislike this behaviour change - but I might be wrong, haha.
Comment Actions
Well, this only changes the behavior in the case of a single word that's wider than the diff window; if it can avoid breaking a word by wrapping it still will. Does your text editor not do the same? I just tested in vim, emacs, TextMate, and Atom and all of them seem to break long words when necessary.
Comment Actions
One technical issue in two places:
- PhabricatorSourceCodeView also uses phabricator-oncopy, but it looks like it wasn't updated, so I'd expect this to break copying text in Paste -- switch the ZWS in PhabricatorSourceCodeView to Word Joiner.
- Likewise, DiffusionBrowseFileController has a copy of this character too. Update that one.
A couple minor things inline.
Also, some browsers (Chromium only?) have unusual behavior for <zero width space> <tab literal> (see https://secure.phabricator.com/T2495#26). This might interact in those cases, but hopefully by accidentally fixing the issue.
| src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php | ||
|---|---|---|
| 359 | Can we safely remove any of these column classes now? | |
| 441–444 | Do images still display OK? | |
Comment Actions
I can't reproduce the tab problem in Chrome beta on OS X and I don't have Chromium on Ubuntu handy, but hopefully nothing will break terribly there.
| src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php | ||
|---|---|---|
| 359 | Yes! I meant to and forgot. | |
| 441–444 | Oops, they're a little funky. I'll fix. | |
Comment Actions
Thanks! Based on the expert strategy of "poking around for 20 seconds", I don't see this causing any display issues. This is pretty easy to adjust later if we missed something, too.
Comment Actions
I think this mucked up generated changes a little bit -- I get this in Safari if I click to show generated changes on a diff like D8646, for the file __phutil_library_map__.php. I can look at this when I get a chance, but you can repro locally by putting @generated in any file and diffing it if you get there first.
