WrapText + LineNumber, wrong line numbers position

[AlexP11223]

When text wrapping and line numbers are enabled, line numbers are displayed incorrectly (which is very confusing and makes them unusable):

They displayed in the center instead of the beginning of the line, like in other text editors. For example Notepad++:

Tried in current master spellchecking demo, as well as in my project with stable and milestone RichTextFX releases.

    StyleClassedTextArea textArea = new StyleClassedTextArea();
    textArea.setWrapText(true);
    textArea.setParagraphGraphicFactory(LineNumberFactory.get(textArea));

[JordanMartinez]

Since you can implement your own ParagraphGraphicFactory, use LineNumberFactory as a base upon which you implement the alignment you want.

[AlexP11223]

Isn't it a bug? All editors I know (JetBrains IDEs, Github, ...) use top alignment for line numbers.

[JordanMartinez]

Right, but AFAIK, all code editors don't wrap their lines like you are doing.

[AlexP11223]

They all have option to wrap, and it can be useful for non-code text files, some config files. Not sure why would anyone prefer middle alignment for line numbers :)

[JordanMartinez]

Mm.. makes sense. If we just make LNF layout its children at the top, won't that properly lay out the number regardless of wrapping?

[AlexP11223]

Yeah, it should. If I add lineNo.setAlignment(Pos.TOP_CENTER); to LNF apply it works as expected.

btw I thought that it would be possible to use right alignment and remove zeros added for padding, but looks like in that case width would not be the same :(

[JordanMartinez]

Couldn't you use Pos.TOP_RIGHT?

[JordanMartinez]

Or is that what you meant by "I thought that it would be possible to use right alignment?"

[AlexP11223]

Yeah, Currently there is no difference between RIGHT and CENTER because it is padded with 0s (and without 0s width is not the same for all lines).

[JordanMartinez]

One other possibly-related problem is if one scrolls a very long document. When only lines 0-9 (single digit) are displayed, the LNF will probably only display just enough horizontal space for those items. As soon as lines 10-99 (double digits) are displayed, the LNF will increase the horizontal space to account for the extra digit. This will cause an unexpected jump in the rendering of the text as it is suddenly rendered farther to the right. Moreover, this jump will occur as soon as lines 100, 1.000, and 10.000 are rendered.
So, a proper solution should also render the padding correctly no matter how many lines there are in the document, so that the text's horizontal position doesn't jump like that.

[AlexP11223]

You mean the jump that I showed on the screenshot above (#488 (comment))? I don't see any other jumps (spellchecking demo with 12000 lines).

[JordanMartinez]

Yup! Just pointing out that that is also part of the issue the solution would need to address.

[AlexP11223]

When removed padding with zeros? Then maybe a simple solution would be to pad with spaces, since the line number font is monospace.

[JordanMartinez]

Then maybe a simple solution would be to pad with spaces, since the line number font is monospace.

Good idea. However, shouldn't the LNF still work correctly even when one does not want monospace to be used? Then again, would that go far enough outside the scope of this project that a person who wants that should just implement their own LNF?

[AlexP11223]

As far as I can see user cannot change font now, it's just a static private field.

[JordanMartinez]

It's possible by styling its CSS class lineno, as that is applied after setting the default font.

[AlexP11223]

Ah, yes. But if someone really wants a non-monospace font for line numbers, then they can just set format parameter back to zeros.
Of course it can be done vice versa (leave zeros by default, as it is is now), though I guess in most cases spaces are more convenient than zeros. But I guess this decision is not related to the wrapping problem.

Padding with spaces can be implemented using format like this: digits -> "%1$" + digits + "s".

[JordanMartinez]

But if someone really wants a non-monospace font for line numbers, then they can just set format parameter back to zeros.

And in case they don't, they should just create their own LNF?

Padding with spaces can be implemented using format like this: digits -> "%1$" + digits + "s".

So, we should change the Pos to be at the top, add another parameter to the base LNF constructor that allows one to pass in the type of formatting to use (spaces or zeros), and update the regular get(Area) constructor to use the zeros formatting by default. Does that sound about right?
If that fully addresses this issue, will you submit a PR for it?

[AlexP11223]

add another parameter to the base LNF constructor that allows one to pass in the type of formatting

There is already such constructor.
https://github.com/TomasMikula/RichTextFX/blob/master/richtextfx/src/main/java/org/fxmisc/richtext/LineNumberFactory.java#L35

Or you mean to add another constructor with just one character (string) instead of formatting function?

[JordanMartinez]

Closed by #489.

[JordanMartinez]

There is already such constructor.

Sorry, I forgot there was already a constructor for that. It's been a while since I looked at that file. Everything I said before was from memory.