Incorrect cursor selection rendering in ASCII area about qhexview HOT 8 CLOSED

I don't think it is the last character of the line, but the last ascii character of whatever selection.

from qhexview.

I think your suggestion is correct and what you define as redundant is just the standard behaviour.

Try with old and new code to highlight multiple lines.
You will see that the hex selection is always 3 char wide as each char is 2 nibbles and a space.

One could decide that the last space at the end of a line with selection is not part of the selection, but I maybe not.

Try to replace -1 with const int gap = factor == 3 ? -1 : 0; and you will see a bad EOL behaviour.
I think removing -1 is the correct solution.

from qhexview.

The hex selection is always 3 char wide as each char is 2 nibbles and a space.

I see, indeed, this is not a bug but simply how hex area is designed to work.

Nevertheless, aesthetically, I think, it looks better when these two “empty” characters which separate Hex from Ascii areas are not touched by the selection.

Maybe @Dax89 put this “-1” on purpose to deal with this particular case at the end of the line, where only 2 characters instead of 3 need to be selected, forgetting that it messes up the Ascii area?

I see that applySelection() is called in 2 places, by drawHex() and drawAscii(). Perhaps “-1” could be reintroduced as some optional argument, used from the drawHex() call only?

Plus with this “-1” selection in hex area seems more natural. That is, it follows mouse cursor precisely. In the version without the “-1” the selection cursor is always one character ahead of the mouse cursor, which feels slightly odd.

from qhexview.

What do you want to happen when the selection spans multiple Hex lines?
Do you want the last (empty) character of a line to appear selected or not?

If you don't want it to be selected, then one must change this too

Is this the rule you have in mind? only ever select the space if in between selected characters and never at the end of a line.

this is VSCode.

from qhexview.

... only ever select the space if in between selected characters and never at the end of a line.

this is VSCode.

Pretty much this, and the screenshot illustrates it too. I think it looks better this way.

from qhexview.

What about this?

I've updated the PR.

from qhexview.

Perfect, thank you!

The only small thing I’d change is removing braces after the if (factor == 3) line:

https://github.com/Dax89/QHexView/pull/68/files#diff-7a1b952c3037a8344622cab0089888627de0af77157b95150caddd67ea49939f#L289-L292

https://github.com/audetto/QHexView/blob/0b7ca06a77e17b3713477a877c3d1c5890ae3b5f/document/qhexrenderer.cpp#L289-L292

And while we’re at it, perhaps the factor argument to applySelection could be enum instead of int?

enum Factor {Ascii = 1, Hex = 3}

So one could write if (factor == Hex) without the need to write the comment, and the function could be called this way instead applySelection (textcursor, line, Hex)

from qhexview.

I see that my previous small suggestion has made it to the code as well, the issue is now fully resolved, thanks!

from qhexview.