Implements selecting entire line of text with a single click on line number.
FEATURE: 372503
ngraham | |
cullmann |
Frameworks | |
Kate | |
KTextEditor |
Implements selecting entire line of text with a single click on line number.
FEATURE: 372503
Click on a line number and check that it selects the line the same way that triple-clicking did before.
Check that selecting a line and then Shift+Click on another line selects all intervening lines.
No Linters Available |
No Unit Test Coverage |
Gave it a whirl; this is very nice!
I don't think making it optional and off by default is the best choice, though. Nobody will ever find it. I would support making this enabled by default at the very least, and possibly even not turn-off-able with an option. Most advanced text editors already have this feature on by default with no option to disable it, and it's really hard for me to imagine what kind of workflow it would would negatively impact. After all, the line numbers themselves are off by default! Good defaults are important, and highly relevant to T6831: Top-notch usability and productivity for basic software.
If you think it should be on by default, that's easily done. I know what you mean, that nobody would ever find it.
When I first started using Kate, I thought the triple-clicking was very annoying. But I got used to it and now it's automatic for me. It also became automatic for me to move the cursor to the start of a line by clicking the line number. When I started using this patch, I found I was always selecting a line when I just wanted to move the cursor. That's why I hesitated to make such a big change to this workflow without making it optional.
IMHO, a mouse-based workflow for selecting large blocks of text makes sense, which is why this should be made at least the default setting. But using the mouse to go to the first column makes less sense, since you're probably about to do something with the text, which means your fingers need to be on the keyboard, which means you should just use the default keyboard shortcut (Home).
I'm okay with making it the default if we do have to add an option. But every option must be carefully considered, or else configuration windows become intimidating and impossible to navigate, and the genuinely useful options become buried. Everything that can be turned on or off must be that way because there is a genuinely useful workflow or a reasonable user preference attached to each setting. We shouldn't add options just to be conservative and avoid blame in case someone doesn't like it. Every change has someone who doesn't like it; completely avoiding that is sadly impossible. Instead, we should try to do the most good for the most people.
I think consistency with the common interaction method is important here: other text editors implement this via single-click. No sense replacing one nonstandard UI with another one.
With the double click option, it allows for two different functionality (place at home row, and highlight) as opposed to one if there was only 1 click to highlight. The previous functionality would be lost with the new single-click.
I haven't really tried the current one click feature until seeing this diff but it's pretty cool.
And actually, if you have the line modification marker border turned on, clicking in it will just move the cursor without selecting the line.
@richardbowen, this isn't a support forum, it's for patch review. Please only leave comments related to the patch itself. Thanks!
@ngraham Qt Designer made those changes when I added the checkbox. They will be gone with the update. The four files concerning the config option will be as they were before this revision.
Removed the config option to enable and disable the single-click behavior.
Zeroing m_scrollX and m_scrollY in beginSelectLine is not relevant to line selection. Removed these lines.
the "also" in your commit message: can you split this commit into two parts, or is the feature addition also fixing the bug? Put differently: Could you first fix the bug, then add the feature, in separate commits?
src/view/kateviewhelpers.cpp | ||
---|---|---|
2018–2023 | const | |
2019 | can you comment this code, why exclude these areas? | |
2183 | const | |
src/view/kateviewinternal.cpp | ||
2787 | dito comment, what has triple click to do with *begin* of select line? Is this b/c an actual triple click would select the line? |
Added comments and removed code to fix view scrolling inconsistancy
when dragging over line numbers.
I have no strong opinions on that, as I am not really using the line numbers in such a way.
But I think it can't hurt to have this in.
Btw, did anyone test with dynamic word wrap enabled? Does it still work, when a line spans multiple view lines?