`TextFormatter` bugs about terminal.gui HOT 35 OPEN

Is it just me or is vertical text with multiple lines, except when centered horiz, completely broken?

Note, to test this, you have to use this PR which fixes a crash in the scenario and updates the API per above:

#3419

from terminal.gui.

Only the ones in the middle vertically are correct. The ones on the sides are incorrect.

I was not setting tf.Size. Nevermind.

from terminal.gui.

Now, I really did find a bug in TextAlignment.Right when tf.AutoSize = true:

(This is my bug. nevermind again... I can't repro it on v2_develop).

from terminal.gui.

Nevermind again and now mind...

It IS a bug in v2_develop. I just had the test disabled. This fails:

[InlineData ("AB", 1, true, "A")] // BUGBUG: This is wrong, it should be "A"

This works

[InlineData ("AB", 1, true, "")] // BUGBUG: This is wrong, it should be "A"

from terminal.gui.

Nevermind again and now mind...

It IS a bug in v2_develop. I just had the test disabled. This fails:

[InlineData ("AB", 1, true, "A")] // BUGBUG: This is wrong, it should be "A"

This works

[InlineData ("AB", 1, true, "")] // BUGBUG: This is wrong, it should be "A"

@BDisp since you wrote the TextFormatter.Draw code (?), could you look at this and see if you can see how to fix it?

from terminal.gui.

@BDisp since you wrote the TextFormatter.Draw code (?), could you look at this and see if you can see how to fix it?

If you don't mind, I'll look at it tomorrow. Thanks.

from terminal.gui.

A lot of this has already been fixed in code and documentation fixes I currently have outstanding.

I'd suggest not dealing with it right now, until that's done.

from terminal.gui.

I understand your doubt. Maybe you can improve the documentation for this method. In fact, in this case it should return 1, because this method is used for the vertical direction of the text, to obtain the maximum size that a column will occupy in a set of characters. If in a line there is a glyph that occupies two columns then it will return 2. Therefore, it does not return the sum of the column sizes but only the maximum size occupied by a character.

from terminal.gui.

I understand your doubt. Maybe you can improve the documentation for this method. In fact, in this case it should return 1, because this method is used for the vertical direction of the text, to obtain the maximum size that a column will occupy in a set of characters. If in a line there is a glyph that occupies two columns then it will return 2. Therefore, it does not return the sum of the column sizes but only the maximum size occupied by a character.

Yeah, I think I see it now. On it...

from terminal.gui.

This helped me fix issues here:

    /// <summary>
    ///     Returns the number of columns required to render <paramref name="lines"/> oriented vertically.
    /// </summary>
    /// <remarks>
    ///     This API will return incorrect results if the text includes glyphs who's width is dependent on surrounding
    ///     glyphs (e.g. Arabic).
    /// </remarks>
    /// <param name="lines">The lines.</param>
    /// <param name="startLine">The line in the list to start with (any lines before will be ignored).</param>
    /// <param name="linesCount">The number of lines to process (if less than <c>lines.Count</c>, any lines after will be ignored).</param>
    /// <param name="tabWidth">The number of columns used for a tab.</param>
    /// <returns>The width required.</returns>
    public static int GetColumnsRequiredForVerticalText (
        List<string> lines,
        int startLine = -1,
        int linesCount = -1,
        int tabWidth = 0
    )

This was my bad. I renamed this method way back without really understanding what it did.

from terminal.gui.

Glad this helped fix the error. What matters is that now you know its purpose and it will certainly be well documented.

from terminal.gui.

While we're at it, perhaps you could help me understand why this test isn't working right

    [SetupFakeDriver]
    [Theory]
    [InlineData ("A", 0, "")]
    [InlineData ("A", 1, "A")]
    [InlineData ("A", 2, "A")]
    [InlineData ("A B", 3, "A B")]
    [InlineData ("A B", 1, "A")]
    [InlineData ("A B", 2, "A")]
    [InlineData ("A B", 3, "A B")]
    [InlineData ("A B", 4, "A B")] // BUGBUG: This should be "A  B"
    [InlineData ("A B", 5, "A B")]// BUGBUG: This should be "A   B"
    [InlineData ("A B", 6, "A B")]// BUGBUG: This should be "A   B"
    [InlineData ("A B", 10, "A B")]// BUGBUG: This should be "A      B"
    [InlineData ("ABC ABC", 10, "ABC ABC")] // BUGBUG: this is wrong
    [InlineData ("ABC ABC", 12, "ABC ABC")]  // BUGBUG: this is wrong
    public void Draw_Horizontal_Justified (string text, int width, string expectedText)
    {
        TextFormatter tf = new ()
        {
            Text = text,
            Alignment = TextAlignment.Justified,
        };
        tf.Draw (new Rectangle (0, 0, width, 1), Attribute.Default, Attribute.Default);

        TestHelpers.AssertDriverContentsWithFrameAre (expectedText, _output);
    }

from terminal.gui.

Only the ones in the middle vertically are correct. The ones on the sides are incorrect.

from terminal.gui.

Now, I really did find a bug in TextAlignment.Right when tf.AutoSize = true:

    [SetupFakeDriver]
    [Theory]
    [InlineData ("A", 0, false, "")]
    [InlineData ("A", 1, false, "A")]
    [InlineData ("A", 2, false, " A")]
    [InlineData ("AB", 1, false, "A")]
    [InlineData ("AB", 2, false, "AB")]
    [InlineData ("ABC", 3, false, "ABC")]
    [InlineData ("ABC", 4, false, " ABC")]
    [InlineData ("ABC", 6, false, "   ABC")]

    [InlineData ("A", 0, true, "")]
    [InlineData ("A", 1, true, "A")]
    [InlineData ("A", 2, true, " A")]
    [InlineData ("AB", 1, true, "")] // BUGBUG: This is wrong, it should be "A"
    [InlineData ("AB", 2, true, "AB")]
    [InlineData ("ABC", 3, true, "ABC")]
    [InlineData ("ABC", 4, true, " ABC")]
    [InlineData ("ABC", 6, true, "   ABC")]
    public void Draw_Horizontal_Right (string text, int width, bool autoSize, string expectedText)
    {
        TextFormatter tf = new ()
        {
            Text = text,
            Alignment = TextAlignment.Right,
            AutoSize = autoSize,
        };

        if (!autoSize)
        {
            tf.Size = new Size (width, 1);
        }

        tf.Draw (new Rectangle (Point.Empty, new (width, 1)), Attribute.Default, Attribute.Default);
        TestHelpers.AssertDriverContentsWithFrameAre (expectedText, _output);
    }

from terminal.gui.

I've also seen that Button doesn't draw the right brackets in justified alignment in some situations. Have you ever noticed this?

from terminal.gui.

There were a few test cases that were very clearly explicitly defined incorrectly to ignore what were probably deemed minor issues when it was done. Some even had comments explicitly saying that. Those now are defined correctly and pass correctly in what I've got in progress.

from terminal.gui.

If you don't mind, I'll look at it tomorrow. Thanks.

My laptop is broken and only on Tuesday the SSD will be ready. Sorry.

from terminal.gui.

If you don't mind, I'll look at it tomorrow. Thanks.

My laptop is broken and only on Tuesday the SSD will be ready. Sorry.

Bummer!

from terminal.gui.

Luckily it was ready early. Both are not correct. In horizontal alignment to the right, it is always the last letter of a word that is always placed to the right, in the case of left-to-right direction. In this case it will be "B". The following change works:

In the case of the vertical direction of the text, it still seems like you are confusing the function that returns the maximum width of a rune, whose value should be 1, 2 or the tab width and not the sum of all runes. Remember that the Draw method already separates new lines and therefore these functions do not have to deal with them.

from terminal.gui.