Mpdreamz / shellprogressbar

ShellProgressBar - display progress in your console application
MIT License
1.44k stars 134 forks source link

ProgressBar.WriteLine does not work correctly on multiline writes or when lines are longer then ConsoleWidth #102

Open EddyToo opened 1 year ago

EddyToo commented 1 year ago

The DefaultConsoleWrite method assumes that the written height will always be 1 which is obviously not the case when writing multiline strings or when the string contents does not fit on a single line. The effect is that anything beyond the first line will be overwritten

The method also increases the _originalCursorTop with the lines written, ignoring the effect scrolling beyond the size of the buffer has. Since 5 messages can be written for each draw operation it can lead to _originalCursorTop exceeding WindowHeight. This causes different visual issues because the code looses track of where the top of the progressbar was/should be

This issue is related to #79

EddyToo commented 1 year ago

Suggested patch to resolve this issue and #79

diff --git a/src/ShellProgressBar.Example/Examples/MessageBeforeAndAfterExample.cs b/src/ShellProgressBar.Example/Examples/MessageBeforeAndAfterExample.cs
index 4280eef..eaaf98e 100644
--- a/src/ShellProgressBar.Example/Examples/MessageBeforeAndAfterExample.cs
+++ b/src/ShellProgressBar.Example/Examples/MessageBeforeAndAfterExample.cs
@@ -8,7 +8,7 @@ namespace ShellProgressBar.Example.Examples
        protected override Task StartAsync()
        {
            Console.WriteLine("This should not be overwritten");
-           const int totalTicks = 10;
+           int totalTicks = Console.WindowHeight;
            var options = new ProgressBarOptions
            {
                ForegroundColor = ConsoleColor.Yellow,
@@ -18,9 +18,27 @@ namespace ShellProgressBar.Example.Examples
            };
            using (var pbar = new ProgressBar(totalTicks, "showing off styling", options))
            {
-               TickToCompletion(pbar, totalTicks, sleep: 500, i =>
+               TickToCompletion(pbar, totalTicks, sleep: 250, i =>
                {
-                   pbar.WriteErrorLine($"This should appear above:{i}");
+                   if (i % 5 == 0)
+                   {
+                       // Single line
+                       pbar.WriteErrorLine($"[{i}] This{Environment.NewLine}[{i}] is{Environment.NewLine}[{i}] over{Environment.NewLine}[{i}] 4 lines");
+                       return;
+                   }
+                   if (i % 4 == 0)
+                   {
+                       // Single line
+                       pbar.WriteErrorLine($"[{i}] This has{Environment.NewLine}[{i}] 2 lines.");
+                       return;
+                   }
+                   if (i % 3 == 0)
+                   {
+                       // Single line
+                       pbar.WriteErrorLine($"[{i}] This is a very long line {new string('.', Console.BufferWidth)} and should be split over 2 lines");
+                       return;
+                   }
+                   pbar.WriteErrorLine($"[{i}] This should appear above");
                });
            }

diff --git a/src/ShellProgressBar.Example/Program.cs b/src/ShellProgressBar.Example/Program.cs
index b947538..0acc86e 100644
--- a/src/ShellProgressBar.Example/Program.cs
+++ b/src/ShellProgressBar.Example/Program.cs
@@ -66,6 +66,9 @@ namespace ShellProgressBar.Example
                case "test":
                    await RunTestCases(token);
                    return;
+               case "scrolltest":
+                   await RunTestCases(token, Console.WindowHeight+5);
+                   return;
                case "example":
                    var nth = args.Length > 1 ? int.Parse(args[1]) : 0;
                    await RunExample(nth, token);
@@ -88,12 +91,16 @@ namespace ShellProgressBar.Example
            await example.Start(token);
        }

-       private static async Task RunTestCases(CancellationToken token)
+       private static async Task RunTestCases(CancellationToken token, int writeNumOfRowBefore = 0)
        {
            var i = 0;
            foreach (var example in TestCases)
            {
                if (i > 0) Console.Clear(); //not necessary but for demo/recording purposes.
+
+               for (int r = 0; r< writeNumOfRowBefore; r++)
+                   Console.WriteLine($"Writing output before test. Row {r+1}/{writeNumOfRowBefore}");
+
                await example.Start(token);
                i++;
            }
diff --git a/src/ShellProgressBar/ProgressBar.cs b/src/ShellProgressBar/ProgressBar.cs
index a76e525..9208fa8 100644
--- a/src/ShellProgressBar/ProgressBar.cs
+++ b/src/ShellProgressBar/ProgressBar.cs
@@ -2,7 +2,6 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
@@ -15,11 +14,10 @@ namespace ShellProgressBar

        private readonly ConsoleColor _originalColor;
        private readonly Func<ConsoleOutLine, int> _writeMessageToConsole;
-       private readonly int _originalWindowTop;
-       private readonly int _originalWindowHeight;
        private readonly bool _startedRedirected;
        private int _originalCursorTop;
        private int _isDisposed;
+       private int _lastDrawBottomPos;

        private Timer _timer;
        private int _visibleDescendants = 0;
@@ -41,8 +39,6 @@ namespace ShellProgressBar
            try
            {
                _originalCursorTop = Console.CursorTop;
-               _originalWindowTop = Console.WindowTop;
-               _originalWindowHeight = Console.WindowHeight + _originalWindowTop;
                _originalColor = Console.ForegroundColor;
            }
            catch
@@ -56,7 +52,7 @@ namespace ShellProgressBar
            if (this.Options.EnableTaskBarProgress)
                TaskbarProgress.SetState(TaskbarProgress.TaskbarStates.Normal);

-           if (this.Options.DisplayTimeInRealTime)
+           if (this.Options.DisplayTimeInRealTime) 
                _timer = new Timer((s) => OnTimerTick(), null, 500, 500);
            else //draw once
                _timer = new Timer((s) =>
@@ -102,18 +98,22 @@ namespace ShellProgressBar

        private void EnsureMainProgressBarVisible(int extraBars = 0)
        {
+           var lastVisibleRow = Console.WindowHeight + Console.WindowTop;
+
            var pbarHeight = this.Options.DenseProgressBar ? 1 : 2;
-           var neededPadding = Math.Min(_originalWindowHeight - pbarHeight, (1 + extraBars) * pbarHeight);
-           var difference = _originalWindowHeight - _originalCursorTop;
-           var write = difference <= neededPadding ? Math.Max(0, Math.Max(neededPadding, difference)) : 0;
+           var neededPadding = Math.Min(lastVisibleRow - pbarHeight, (1 + extraBars) * pbarHeight);
+           var difference = lastVisibleRow - _originalCursorTop;
+           var write = difference <= neededPadding ? Math.Min(Console.WindowHeight, Math.Max(0, Math.Max(neededPadding, difference))) : 0;
+
+           if (write == 0)
+               return;

            var written = 0;
            for (; written < write; written++)
                Console.WriteLine();
-           if (written == 0) return;

-           Console.CursorTop = _originalWindowHeight - (written);
-           _originalCursorTop = Console.CursorTop - 1;
+           Console.CursorTop = Console.WindowHeight + Console.WindowTop - write;
+           _originalCursorTop = Console.CursorTop -1;
        }

        private void GrowDrawingAreaBasedOnChildren() => EnsureMainProgressBarVisible(_visibleDescendants);
@@ -345,7 +345,12 @@ namespace ShellProgressBar

            DrawChildren(this.Children, indentation, ref cursorTop, Options.PercentageFormat);

-           ResetToBottom(ref cursorTop);
+           if (Console.CursorTop < _lastDrawBottomPos)
+           {
+               // The bar shrunk. Need to clean the remaining rows
+               ClearLines(_lastDrawBottomPos - Console.CursorTop);
+           }
+           _lastDrawBottomPos = Console.CursorTop;

            Console.SetCursorPosition(0, _originalCursorTop);
            Console.ForegroundColor = _originalColor;
@@ -355,35 +360,60 @@ namespace ShellProgressBar
            _timer = null;
        }

+       private static void ClearLines(int numOfLines)
+       {
+           // Use bufferwidth and not only the visible width. (currently identical on all platforms)
+           Console.Write(new string(' ', Console.BufferWidth * numOfLines));
+       }
+
+       private static string _resetString = "";
+       private static void ClearCurrentLine()
+       {
+           if (_resetString.Length != Console.BufferWidth + 2)
+           {
+               // Use buffer width and not only the visible width. (currently identical on all platforms)
+               _resetString = $"\r{new string(' ', Console.BufferWidth)}\r";
+           }
+           Console.Write(_resetString);
+       }
+
        private void WriteConsoleLine(ConsoleOutLine m)
        {
-           var resetString = new string(' ', Console.WindowWidth);
-           Console.Write(resetString);
-           Console.Write("\r");
            var foreground = Console.ForegroundColor;
            var background = Console.BackgroundColor;
-           var written = _writeMessageToConsole(m);
+           ClearCurrentLine();
+           var moved = _writeMessageToConsole(m);
            Console.ForegroundColor = foreground;
            Console.BackgroundColor = background;
-           _originalCursorTop += written;
+           _originalCursorTop += moved;
        }

        private static int DefaultConsoleWrite(ConsoleOutLine line)
        {
-           if (line.Error) Console.Error.WriteLine(line.Line);
-           else Console.WriteLine(line.Line);
-           return 1;
-       }
+           var fromPos = Console.CursorTop;

-       private void ResetToBottom(ref int cursorTop)
-       {
-           var resetString = new string(' ', Console.WindowWidth);
-           var windowHeight = _originalWindowHeight;
-           if (cursorTop >= (windowHeight - 1)) return;
-           do
+           // First line was already cleared by WriteConsoleLine().
+           // Would be cleaner to do it here, but would break backwards compatibility for those
+           // who implemented their own writer function.
+           bool isClearedLine = true;
+           foreach (var outLine in line.Line.SplitToConsoleLines())
            {
-               Console.Write(resetString);
-           } while (++cursorTop < (windowHeight - 1));
+               // Skip slower line clearing if we scrolled on last write
+               if (!isClearedLine)
+                   ClearCurrentLine();
+
+               int lastCursorTop = Console.CursorTop;
+               if (line.Error)
+                   Console.Error.WriteLine(outLine);
+               else
+                   Console.WriteLine(outLine);
+
+               // If the cursorTop is still on same position we are at the end of the buffer and scrolling happened.
+               isClearedLine = lastCursorTop == Console.CursorTop;
+           }
+
+           // Return how many rows the cursor actually moved by.
+           return Console.CursorTop - fromPos;
        }

        private static void DrawChildren(IEnumerable<ChildProgressBar> children, Indentation[] indentation,
@@ -392,12 +422,12 @@ namespace ShellProgressBar
            var view = children.Where(c => !c.Collapse).Select((c, i) => new {c, i}).ToList();
            if (!view.Any()) return;

-           var windowHeight = Console.WindowHeight;
+           var lastVisibleRow = Console.WindowHeight + Console.WindowTop;
            var lastChild = view.Max(t => t.i);
            foreach (var tuple in view)
            {
-               //Dont bother drawing children that would fall off the screen
-               if (cursorTop >= (windowHeight - 2))
+               // Dont bother drawing children that would fall off the screen and don't want to scroll top out of view
+                if (cursorTop >= (lastVisibleRow - 2))
                    return;

                var child = tuple.c;
@@ -500,7 +530,7 @@ namespace ShellProgressBar
            {
                var pbarHeight = this.Options.DenseProgressBar ? 1 : 2;
                var openDescendantsPadding = (_visibleDescendants * pbarHeight);
-               var newCursorTop = Math.Min(_originalWindowHeight, _originalCursorTop + pbarHeight + openDescendantsPadding);
+               var newCursorTop = Math.Min(Console.WindowHeight+Console.WindowTop, _originalCursorTop + pbarHeight + openDescendantsPadding);
                Console.CursorVisible = true;
                Console.SetCursorPosition(0, newCursorTop);
            }
diff --git a/src/ShellProgressBar/StringExtensions.cs b/src/ShellProgressBar/StringExtensions.cs
index bc3071b..0d00102 100644
--- a/src/ShellProgressBar/StringExtensions.cs
+++ b/src/ShellProgressBar/StringExtensions.cs
@@ -12,5 +12,32 @@ namespace ShellProgressBar
                 return phrase;
             return phrase.Substring(0, length - 3) + "...";
         }
+
+        /// <summary>
+        /// Splits a string into it's indiviudal lines and then again splits these individual lines
+        /// into multiple lines if they exceed the width of the console.
+        /// </summary>
+        /// <param name="str"></param>
+        /// <returns></returns>
+        public static IEnumerable<string> SplitToConsoleLines(this string str)
+        {
+           int width = Console.BufferWidth;
+           var lines = str.Split(new string[] { Environment.NewLine }, StringSplitOptions.None);
+
+           foreach (var line in lines)
+           {
+               if (line.Length > width)
+               {
+                   for (int i = 0; i < line.Length; i += width)
+                   {
+                       yield return line.Substring(i, Math.Min(width, line.Length - i));
+                   }
+               }
+               else
+               {
+                   yield return line;
+               }
+           }
+        }
     }
 }
KoalaBear84 commented 1 year ago

It would be great if issues with this are being resolved, as it is almost unusable to use the WriteLine. I was glad I saw the ProgressBar.WriteLine method, but after that already smashed to the floor again because it isn't usable 😂

tishige commented 1 year ago

Thank you!!