Differential D16578

Don't consider a colon before whitespace to be part of a word
ClosedPublic
Actions

Authored by ngraham on Nov 1 2018, 4:23 AM.

Details

Reviewers
hindenburg
Group Reviewers
Konsole
Commits
R319:9ce051371dcf: Don't consider a colon before whitespace to be part of a word
Summary

Currently, colons are considered to be part of words for the purpose of
double-click selection. This allows, for example, URLs to be selected
with a double click. This is good.

However, including colons works less well for paths that end with a colon
and whitespace--such as grep output. Removing the colon from the list
of word characters isn't a great solutionsince then URLs can no longer be
selected by double-clicking.

This patch resolves that particular issue by always omitting a colon from
double-click selection when it's immediately followed by whitespace.

BUG: 399291
FIXED-IN: 18.12.0

Test Plan

grep -ri charClass .
Then double-click on one of the paths in the output. The trailing colon
is not included in the selection.

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Nov 1 2018, 4:23 AM
Restricted Application added a project: Konsole. · View Herald TranscriptNov 1 2018, 4:23 AM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
ngraham requested review of this revision.Nov 1 2018, 4:23 AM
ngraham added a comment.Nov 1 2018, 1:46 PM

If this does not prove popular, I thought up an alternative approach last night: even when the colon character is not in the list of word characters, consider it a word character when it's a part of a string that Konsole detects as a URL.

hindenburg added a comment.Nov 1 2018, 10:17 PM

I don't have an issue w/ the idea. xterm and terminator include the ':', while gnome-terminal and Mac OSX's terminal do not.

ngraham added a comment.Nov 1 2018, 10:39 PM

Cool. I might also implement the other idea in a separate patch since it does improve the behavior when ':' is removed from the words list anyway.

ngraham added a comment.Nov 1 2018, 10:41 PM

Or I could even do it as a part of this patch if you think that would be a good idea (it would be touching the same pieces of code).

hindenburg added a comment.Nov 1 2018, 11:31 PM

We might want to keep them separate in case of issues

ngraham added a comment.Nov 2 2018, 2:27 AM

OK, no problem. I'll submit that one after this lands, if you decide to accept it (or in its place, if you don't :) )

hindenburg accepted this revision.Nov 2 2018, 1:50 PM
This revision is now accepted and ready to land.Nov 2 2018, 1:50 PM
hindenburg edited the summary of this revision. (Show Details)Nov 2 2018, 1:51 PM
hindenburg edited the test plan for this revision. (Show Details)
Closed by commit R319:9ce051371dcf: Don't consider a colon before whitespace to be part of a word (authored by ngraham, committed by hindenburg). · Explain WhyNov 2 2018, 1:51 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/TerminalDisplay.cpp (8 lines)

Diff 44705

View Options

src/TerminalDisplay.cpp

Loading...