After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 725909 - Selecting text can't be started at an empty line
Selecting text can't be started at an empty line
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.35.x
Other Linux
: Normal minor
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-07 18:12 UTC by Egmont Koblinger
Modified: 2015-01-19 19:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix triple click. (1.05 KB, patch)
2015-01-16 19:04 UTC, Egmont Koblinger
committed Details | Review
Fix single click, putty's way (meh) (1.52 KB, patch)
2015-01-16 21:50 UTC, Egmont Koblinger
none Details | Review
Fix single click, xterm's way (yay!) (1.72 KB, patch)
2015-01-16 21:50 UTC, Egmont Koblinger
none Details | Review
Fix single click, xterm's way (yay!) - v2 (1.88 KB, patch)
2015-01-17 00:14 UTC, Egmont Koblinger
committed Details | Review

Description Egmont Koblinger 2014-03-07 18:12:51 UTC
Highlighting text can't be started at an empty line, or if there are multiple consecutive empty lines then it can't be started at the first one.
Comment 1 Egmont Koblinger 2015-01-12 20:32:42 UTC
https://bugzilla.redhat.com/show_bug.cgi?id=1180987 is strongly related; probably a different symptom caused by the same bug.
Comment 2 Egmont Koblinger 2015-01-15 12:09:59 UTC
Also related: bug 622093 and bug 724253.

Seems like a rewrite of selection would be easier than understanding the current code and fixing all these :)

It's the kind of issue I'd really like to see fixed, but don't feel like fixing it myself :D Any volunteers out there? :D
Comment 3 Egmont Koblinger 2015-01-15 12:40:40 UTC
What should be the desired behavior for a triple click: highlight a physical line, or a logical paragraph?

Currently vte highlights logical paragraphs.
- This is more consistent with double click which is also smart at line boundaries: crosses to an adjacent line if and only if an implicit overflow occurred there rather than an explicit newline.
- It'd be weird if a double click highlighted something in an adjacent line and then the 3rd click removed that portion of the highlight.
- This also seems to be more logical to me, as the user's probably not interested in which part of a longer message happened to fit in a single line at a particular terminal width. 
- This is what firefox does, see e.g. the RedHat bugzilla. (Gnome's bugzilla inserts explicit linebreaks on the server side.)

So I'd vote for keeping this approach.

This kinda explains the weird behavior reported in RH BZ. When triple clicking somewhere to the right of a shell prompt, it's actually on an empty area between two paragraphs, so it's technically logical to highlight both adjacent paragraphs. Although it's definitely not what a user would expect. The implemented specification should be: highlight the complete paragraph that belongs to the row where the tripleclick happens.
Comment 4 Trevor Cordes 2015-01-15 13:38:55 UTC
(In reply to comment #3)
> What should be the desired behavior for a triple click: highlight a physical
> line, or a logical paragraph?

No no no.  No terminal tc's a paragraph (assuming I'm understanding your concept of "paragraph" correctly, see below)!  That would be horrible.  Here's 2 good reasons:

1. The (vast) majority of terminal use is done by admins & programmers.  The idea of a "paragraph" is useless to an admin in a .conf file or a programmer in vi editing some C or perl.

2. Anyone who cares about the concept of a paragraph, like when composing an essay, is using a GUI/WYSIWYG editor like OOo.

Sure, some people break these rules, but I'd bet in most cases this is true.  Besides, if you're savvy enough to edit prose in an editor, you already know about your editor's paragraph manipulation and aren't using the mouse for that anyhow.

Why not do what every single terminal in the world has always done, including g-t, and just select the current line?  Surely that is easier to program anyhow.

Now, if you're strictly talking about "wrapped output" being a "paragraph", like when you cat a very long line from a log file, then, sure if you want to select across 2 lines because you know there wasn't a hard NL, then a) that is useful, and b) I believe g-t used to do that.  Seriously, the old g-t behavior was perfect for a decade, why change it?  It's only the broken bits that need to be fixed, mostly having to do with "empty space" and NLs.
Comment 5 Egmont Koblinger 2015-01-15 13:51:33 UTC
> Now, if you're strictly talking about "wrapped output" being a "paragraph",

Yes, I am.

I'm sure all the fullscreen apps terminate each line with an explicit newline, meaning every paragraph will be 1-line high.

Check the current behavior, e.g. cat /proc/cpuinfo, and then triple-click on the flags line. I don't think this behavior is bad, and I don't see any reason why selecting 1 line of that would ever be useful.

That is, my proposal wouldn't change anything, apart from fixing the current bugs.
Comment 6 Trevor Cordes 2015-01-15 13:56:06 UTC
Yes, 100% agreed.  I checked and all the other terms I have installed all select the entire flags "paragraph" of /proc/cpuinfo, and that is the behavior one would expect.  To select only up to "sse2" for instance would be wrong, yes.

Sorry for misunderstanding your paragraph comment before.  You are right, most curses apps would probably put hard NLs in, thus not causing any problems.

Yes, it's just the little bugs introduced in the last year or so that need to be worked on.  I haven't looked at the source yet, I'm not really a C guy, but I can start taking a look.
Comment 7 Egmont Koblinger 2015-01-15 14:02:25 UTC
> I checked and all the other terms I have installed all
> select the entire flags "paragraph" of /proc/cpuinfo

Glad to hear it, I haven't checked them.

> Sorry for misunderstanding your paragraph comment before.

No worries :) It's a notion we made up when implementing rewrap, so we vte developer know what it means :)

> I haven't looked at the source yet, I'm not really a C guy, but
> I can start taking a look.

If you're looking for a challenge, feel free to go ahead, it's up to you :) I don't want to push you, nor to stop you. I myself found the code hard to understand (vte.c, search for "selection"), but maybe I just need to sit down for an hour or two and focus on this.
Comment 8 Egmont Koblinger 2015-01-16 19:04:44 UTC
Created attachment 294711 [details] [review]
Fix triple click.
Comment 9 Egmont Koblinger 2015-01-16 21:49:43 UTC
Returning to the original issue:

It does not only occur with empty lines, in fact, empty lines are not any different than the unused part of any line.

In any line, click at the unused rightmost part, move the mouse to the right: that empty part is highlighted and correspondingly the selection will be "\n" if you stop here.  But if you move the mouse downwards, this "\n" will be dropped and the selection will start with the next line.

From now on in this comment, let's single click on the unused right hand part of a line, move the mouse vertically by N lines, still ending up on some unused area.

(This is something I do very often to highlight complete lines, as triple clicking is inconvenient for me.  Actually my mouse button is broken so I use the touchpad where a single tap+drag is just moves the mouse, so double tap is equivalent to click, triple tap is double click, and quadruple tap is triple click :D It's really crazy.)

In Gtk+, such an operation results in N lines being the selection, starting with a newline, and not ending with one.

This behavior would probably be very bad for terminal emulators due to \n's traditional meaning of being the terminator; in fact, none of the terminals I've tried (incl. vte) does this. The end of the selection is always extended to include the trailing '\n'.  I wouldn't want to change this, so there's no point aiming for mimicking Gtk+.

Some terminal emulators (e.g. putty, st, konsole, terminology - although in konsole and terminology the visual highlight is done differently) only do this, and behave like Gtk+ at the start of the selection.  This way the buffer ends up containing a newline both before and after the content - a total number of N+1 newlines for N lines of content.  I don't like this behavior too much, and IMHO looks stupid when highlighting.

Some other terminal emulators (xterm, urxvt) try to fix the start too, not to include a newline there.  This is what I believe current vte tries to do too, but does it wrong - it doesn't leave you a way to start with the newline, even if you do want that.  And in the special case of an empty line, that'll just be completely skipped.  Both xterm and urxvt choose the following interesting approach to fix this:  If the start of the highlight is on the first unused cell of the row then include the newline, otherwise (if it's further to the right) omit it.  I like this behavior much better.

I have a patch for both (because I first implemented the one I didn't like that much).
Comment 10 Egmont Koblinger 2015-01-16 21:50:27 UTC
Created attachment 294712 [details] [review]
Fix single click, putty's way (meh)
Comment 11 Egmont Koblinger 2015-01-16 21:50:53 UTC
Created attachment 294713 [details] [review]
Fix single click, xterm's way (yay!)
Comment 12 Egmont Koblinger 2015-01-16 23:36:46 UTC
My preferred single click patch breaks triple click again. Damn! :D
Comment 13 Egmont Koblinger 2015-01-17 00:14:53 UTC
Created attachment 294719 [details] [review]
Fix single click, xterm's way (yay!) - v2

v2 - Don't break triple click :)
Comment 14 Egmont Koblinger 2015-01-18 16:57:33 UTC
Both issues are fixed now.  For the single click, I submitted the xterm variant.
Comment 15 Egmont Koblinger 2015-01-19 19:18:54 UTC
I removed the find_start_column method which seemed to be a no-op. I was wrong, it broke backwards selection over a wide char or tab.  Submitting the fix right away.