Skip to content

Commit

Permalink
Fixes #3011. TextFormatter doesn't handle combining and tab runes. (#…
Browse files Browse the repository at this point in the history
…3012)

* Add horizontal and vertical support for combining glyphs.

* Fix text and auto size behavior.

* Add TabWidth property.

* Add unit test for WordWrap.

* Add MultiLine property and improve more code.

* Fix word wrap on MessageBox.

* Fix label unit test.

* Rename to GetTextFormatterSizeNeededForTextAndHotKey

* Proves that TextFormatter.Size not must to have the same View.Bounds.Size.

* Fix fails unit tests.

* Updates AutoSize document.

* Updates MultiLine document.

* Removes Application dependency from the TextFormatter class.

* Fix Draw XML comment.
  • Loading branch information
BDisp committed Nov 26, 2023
1 parent 8ed1b16 commit 3f4d96b
Show file tree
Hide file tree
Showing 8 changed files with 829 additions and 245 deletions.
410 changes: 297 additions & 113 deletions Terminal.Gui/Text/TextFormatter.cs

Large diffs are not rendered by default.

24 changes: 18 additions & 6 deletions Terminal.Gui/View/ViewLayout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public virtual Rect Frame {
_frame = new Rect (value.X, value.Y, Math.Max (value.Width, 0), Math.Max (value.Height, 0));
if (IsInitialized || LayoutStyle == LayoutStyle.Absolute) {
LayoutFrames ();
TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();
SetNeedsLayout ();
SetNeedsDisplay ();
}
Expand Down Expand Up @@ -213,9 +213,11 @@ public virtual Rect Bounds {
#if DEBUG
if (LayoutStyle == LayoutStyle.Computed && !IsInitialized) {
Debug.WriteLine ($"WARNING: Bounds is being accessed before the View has been initialized. This is likely a bug. View: {this}");
Debug.WriteLine ($"The Frame is set before the View has been initialized. So it isn't a bug.Is by design.");
}
#endif // DEBUG
var frameRelativeBounds = Padding?.Thickness.GetInside (Padding.Frame) ?? new Rect (default, Frame.Size);
//var frameRelativeBounds = Padding?.Thickness.GetInside (Padding.Frame) ?? new Rect (default, Frame.Size);
var frameRelativeBounds = FrameGetInsideBounds ();
return new Rect (default, frameRelativeBounds.Size);
}
set {
Expand All @@ -229,6 +231,16 @@ public virtual Rect Bounds {
}
}

private Rect FrameGetInsideBounds ()
{
if (Margin == null || Border == null || Padding == null) {
return new Rect (default, Frame.Size);
}
var width = Math.Max (0, Frame.Size.Width - Margin.Thickness.Horizontal - Border.Thickness.Horizontal - Padding.Thickness.Horizontal);
var height = Math.Max (0, Frame.Size.Height - Margin.Thickness.Vertical - Border.Thickness.Vertical - Padding.Thickness.Vertical);
return new Rect (Point.Empty, new Size (width, height));
}

// Diagnostics to highlight when X or Y is read before the view has been initialized
private Pos VerifyIsIntialized (Pos pos)
{
Expand Down Expand Up @@ -460,7 +472,7 @@ protected virtual void OnResizeNeeded ()
if (IsInitialized || LayoutStyle == LayoutStyle.Absolute) {
SetMinWidthHeight ();
LayoutFrames ();
TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();
SetNeedsLayout ();
SetNeedsDisplay ();
}
Expand Down Expand Up @@ -657,7 +669,7 @@ int CalculateNewDimension (Dim d, int location, int dimension, int autosize)
Frame = r;
// BUGBUG: Why is this AFTER setting Frame? Seems duplicative.
if (!SetMinWidthHeight ()) {
TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();
}
}
}
Expand Down Expand Up @@ -878,7 +890,7 @@ public virtual void LayoutSubviews ()
var oldBounds = Bounds;
OnLayoutStarted (new LayoutEventArgs () { OldBounds = oldBounds });

TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();

// Sort out the dependencies of the X, Y, Width, Height properties
var nodes = new HashSet<View> ();
Expand Down Expand Up @@ -958,7 +970,7 @@ bool ResizeView (bool autoSize)
}
}
// BUGBUG: This call may be redundant
TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();
return aSize;
}

Expand Down
11 changes: 5 additions & 6 deletions Terminal.Gui/View/ViewText.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private void UpdateTextDirection (TextDirection newDirection)
} else {
SetMinWidthHeight ();
}
TextFormatter.Size = GetSizeNeededForTextAndHotKey ();
TextFormatter.Size = GetTextFormatterSizeNeededForTextAndHotKey ();
SetNeedsDisplay ();
}

Expand All @@ -159,7 +159,7 @@ public int GetHotKeySpecifierLength (bool isWidth = true)
} else {
return TextFormatter.IsVerticalDirection (TextDirection) &&
TextFormatter.Text?.Contains ((char)HotKeySpecifier.Value) == true
? Math.Max (HotKeySpecifier.GetColumns(), 0) : 0;
? Math.Max (HotKeySpecifier.GetColumns (), 0) : 0;
}
}

Expand All @@ -177,7 +177,7 @@ public Size GetSizeNeededForTextWithoutHotKey ()
/// Gets the dimensions required for <see cref="Text"/> accounting for a <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/> .
/// </summary>
/// <returns></returns>
public Size GetSizeNeededForTextAndHotKey ()
public Size GetTextFormatterSizeNeededForTextAndHotKey ()
{
if (string.IsNullOrEmpty (TextFormatter.Text)) {

Expand All @@ -188,9 +188,8 @@ public Size GetSizeNeededForTextAndHotKey ()

// BUGBUG: This IGNORES what Text is set to, using on only the current View size. This doesn't seem to make sense.
// BUGBUG: This uses Frame; in v2 it should be Bounds
return new Size (_frame.Size.Width + GetHotKeySpecifierLength (),
_frame.Size.Height + GetHotKeySpecifierLength (false));
return new Size (Bounds.Size.Width + GetHotKeySpecifierLength (),
Bounds.Size.Height + GetHotKeySpecifierLength (false));
}

}
}
26 changes: 14 additions & 12 deletions Terminal.Gui/Views/MessageBox.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using System.Text;
using System;
using System;
using System.Collections.Generic;
using Terminal.Gui;
using static Terminal.Gui.ConfigurationManager;

namespace Terminal.Gui {
Expand Down Expand Up @@ -262,7 +260,7 @@ static int QueryFull (bool useErrorColors, int width, int height, string title,
}

}

Dialog d;
d = new Dialog (buttonList.ToArray ()) {
Title = title,
Expand All @@ -273,37 +271,40 @@ static int QueryFull (bool useErrorColors, int width, int height, string title,

if (width != 0) {
d.Width = width;
}
}

if (height != 0) {
d.Height = height;
}

if (useErrorColors) {
d.ColorScheme = Colors.Error;
} else {
d.ColorScheme = Colors.Dialog;
}

var messageLabel = new Label () {
AutoSize = false,
AutoSize = wrapMessage ? false : true,
Text = message,
TextAlignment = TextAlignment.Centered,
X = 0,
Y = 0,
Width = Dim.Fill (0),
Height = Dim.Fill (1)
};
messageLabel.TextFormatter.WordWrap = wrapMessage; // BUGBUG: This does nothing as it's not implemented by TextFormatter!
messageLabel.TextFormatter.WordWrap = wrapMessage;
messageLabel.TextFormatter.MultiLine = wrapMessage ? false : true;
d.Add (messageLabel);

d.Loaded += (s, e) => {
if (width != 0 || height != 0) {
return;
}
// TODO: replace with Dim.Fit when implemented
var maxBounds = d.SuperView?.Bounds ?? Application.Top.Bounds;
messageLabel.TextFormatter.Size = new Size (maxBounds.Size.Width - d.GetFramesThickness ().Horizontal, maxBounds.Size.Height - d.GetFramesThickness ().Vertical);
if (wrapMessage) {
messageLabel.TextFormatter.Size = new Size (maxBounds.Size.Width - d.GetFramesThickness ().Horizontal, maxBounds.Size.Height - d.GetFramesThickness ().Vertical);
}
var msg = messageLabel.TextFormatter.Format ();
var messageSize = messageLabel.TextFormatter.GetFormattedSize ();
Expand All @@ -314,7 +315,8 @@ static int QueryFull (bool useErrorColors, int width, int height, string title,
d.Width = newWidth;
}
// Ensure height fits the text + vspace + buttons
d.Height = Math.Max (height, messageSize.Height + 2 + d.GetFramesThickness ().Vertical);
var lastLine = messageLabel.TextFormatter.Lines [^1];
d.Height = Math.Max (height, messageSize.Height + (lastLine.EndsWith ("\r\n") || lastLine.EndsWith ('\n') ? 1 : 2) + d.GetFramesThickness ().Vertical);
d.SetRelativeLayout (d.SuperView?.Frame ?? Application.Top.Frame);
};

Expand Down
108 changes: 59 additions & 49 deletions UnitTests/Dialogs/MessageBoxTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -439,40 +439,40 @@ public void Message_Without_Spaces_WrapMessage_False ()
iterations++;
if (iterations == 0) {
MessageBox.Query (string.Empty, new string ('f', 50), defaultButton: 0, wrapMessage: true, "btn");
MessageBox.Query (string.Empty, new string ('f', 50), defaultButton: 0, wrapMessage: false, "btn");
Application.RequestStop ();
} else if (iterations == 1) {
Application.Refresh ();
TestHelpers.AssertDriverContentsWithFrameAre (@$"
╔══════════════════╗
┌────────────────┐
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ ff │║
║│ │║
{btn}
└────────────────┘
────────────────────
ffffffffffffffffffff
⟦► btn ◄⟧
────────────────────
╚══════════════════╝", output);
Application.RequestStop ();
// Really long text
MessageBox.Query (string.Empty, new string ('f', 500), defaultButton: 0, wrapMessage: true, "btn");
MessageBox.Query (string.Empty, new string ('f', 500), defaultButton: 0, wrapMessage: false, "btn");
} else if (iterations == 2) {
Application.Refresh ();
TestHelpers.AssertDriverContentsWithFrameAre (@$"
┌────────────────┐
│ffffffffffffffff│
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
│ffffffffffffffff│
{btn}
└────────────────┘", output);
══════════════════
────────────────────
ffffffffffffffffffff
⟦► btn ◄⟧
────────────────────
══════════════════", output);
Application.RequestStop ();
}
Expand Down Expand Up @@ -505,14 +505,14 @@ public void Message_With_Spaces_WrapMessage_False ()
Application.Refresh ();
TestHelpers.AssertDriverContentsWithFrameAre (@$"
╔══════════════════╗
┌──────────────┐
║ │ff ff ff ff ff│ ║
║ │ff ff ff ff ff│ ║
║ │ff ff ff ff ff│ ║
║ │ ff ff │ ║
║ │ │ ║
{btn}
└──────────────┘
────────────────────
ff ff ff ff ff ff ff
⟦► btn ◄⟧
────────────────────
╚══════════════════╝", output);
Application.RequestStop ();
Expand All @@ -521,16 +521,16 @@ public void Message_With_Spaces_WrapMessage_False ()
} else if (iterations == 2) {
Application.Refresh ();
TestHelpers.AssertDriverContentsWithFrameAre (@$"
┌────────────────┐
│ffffffffffffffff│
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
║│ffffffffffffffff│║
│ffffffffffffffff│
{btn}
└────────────────┘", output);
══════════════════
────────────────────
ffffffffffffffffffff
⟦► btn ◄⟧
────────────────────
══════════════════", output);
Application.RequestStop ();
}
};
Expand All @@ -539,15 +539,15 @@ public void Message_With_Spaces_WrapMessage_False ()
}

[Theory, AutoInitShutdown]
[InlineData (" ", true)]
[InlineData (" ", false)]
[InlineData ("", true)]
[InlineData ("", false)]
[InlineData ("\n", true)]
[InlineData ("\n", false)]
[InlineData (" \n", true)]
[InlineData (" \n", false)]
public void Message_Empty_Or_A_NewLline_WrapMessagge_True_Or_False (string message, bool wrapMessage)
[InlineData (" ", true, 1)]
[InlineData (" ", false, 1)]
[InlineData ("", true, 1)]
[InlineData ("", false, 1)]
[InlineData ("\n", true, 1)]
[InlineData ("\n", false, 1)]
[InlineData (" \n", true, 1)]
[InlineData (" \n", false, 2)]
public void Message_Empty_Or_A_NewLline_WrapMessagge_True_Or_False (string message, bool wrapMessage, int linesLength)
{
var iterations = -1;
Application.Begin (Application.Top);
Expand All @@ -561,17 +561,27 @@ public void Message_Empty_Or_A_NewLline_WrapMessagge_True_Or_False (string messa
Application.RequestStop ();
} else if (iterations == 1) {
Application.Refresh ();
TestHelpers.AssertDriverContentsWithFrameAre (@$"
if (linesLength == 1) {
TestHelpers.AssertDriverContentsWithFrameAre (@$"
┌──────────────────────────────────────────────┐
│ │
│ │
{CM.Glyphs.LeftBracket}{CM.Glyphs.LeftDefaultIndicator} ok {CM.Glyphs.RightDefaultIndicator}{CM.Glyphs.RightBracket}
└──────────────────────────────────────────────┘", output);
} else {
TestHelpers.AssertDriverContentsWithFrameAre (@$"
┌──────────────────────────────────────────────┐
│ │
│ │
│ │
{CM.Glyphs.LeftBracket}{CM.Glyphs.LeftDefaultIndicator} ok {CM.Glyphs.RightDefaultIndicator}{CM.Glyphs.RightBracket}
└──────────────────────────────────────────────┘", output);
}
Application.RequestStop ();
}
};

Application.Run ();
}
}
}
}
Loading

0 comments on commit 3f4d96b

Please sign in to comment.