MoaidHathot/Dumpify

Question: Is there a way to disable line wrapping?

Closed this issue · 14 comments

hub-stuff commented

In case there is much to dump on a single line I saw values being continued in next line. It would be good to be able to allow that all stuff is rendered to one line only. Below image shows the issue
image

MoaidHathot commented

Hi @hub-stuff, thanks for the suggestion.
I have a follow up question: Does this happen when the rendered table-width is almost equals to your terminal's width?

hub-stuff commented

This happens when object has more parameters nested - I guess.
It can get even more wrapped see below (names are a bit altered but number of chars are the same).
I am using value.DumpText(members: new MembersConfig { IncludeNonPublicMembers = true });

image

MoaidHathot commented

Oh, I see, you are correct- when using DumpText the rendered table's width should not be limited and the lines should not be wrapped by default.
I will take a look into this and update. Thanks @hub-stuff

  • 👍1
  • ❤️1
MoaidHathot commented

Reopening the issue due to a bug, as described in the PR #8

tcortega commented

Unfortunately, this change alone is not enough. #3 isn't fully resolved, and in some cases this change can produce a new bug when NoWrapping is enabled but there is not enough place to output the Table, therefore ... is outputted instead.
In order for #3 to be resolved, we should use a custom IAnsiConsoleOutput and specify big enough Width/Height values, since the defaults are DefaultTerminalWidth which are very small (80 pixels).

This seems to be more of a Spectre.Console problem, look at this issue:
spectreconsole/spectre.console#1185

MoaidHathot commented

That might be the Root cause, and we should monitor the issue in Specture.Console regarding the wrapping. In the meanwhile, the workaround I suggested with the custom IAnsiConsoleOutput works and is very simple to implement. We just need to think of a proper way to determine when to change the default IAnsiConsoleOutput and use the custom one instead.

Something like

internal class LargeConsoleSizeOutput : IAnsiConsoleOutput
{
    public LargeConsoleSizeOutput(TextWriter writer)
        => Writer = writer;

    public void SetEncoding(Encoding encoding)
        => System.Console.OutputEncoding = encoding;

    public TextWriter Writer { get; }
    public bool IsTerminal => false;
    public int Width => 1000;
    public int Height => 1000;
}

Then in SpectreConsoleRenderedObject use

 public void Output(IDumpOutput output)
    {
        var console = AnsiConsole.Create(new AnsiConsoleSettings
        {
            Ansi = AnsiSupport.No,
            ColorSystem = ColorSystemSupport.Detect,
            //Out = new AnsiConsoleOutput(output.TextWriter),
            Out = new LargeConsoleSizeOutput(output.TextWriter),
        });

        console.Write(_renderable);
        console.WriteLine();
    }
tcortega commented

That might be the Root cause, and we should monitor the issue in Specture.Console regarding the wrapping. In the meanwhile, the workaround I suggested with the custom IAnsiConsoleOutput works and is very simple to implement. We just need to think of a proper way to determine when to change the default IAnsiConsoleOutput and use the custom one instead.

Something like

internal class LargeConsoleSizeOutput : IAnsiConsoleOutput
{
    public LargeConsoleSizeOutput(TextWriter writer)
        => Writer = writer;

    public void SetEncoding(Encoding encoding)
        => System.Console.OutputEncoding = encoding;

    public TextWriter Writer { get; }
    public bool IsTerminal => false;
    public int Width => 1000;
    public int Height => 1000;
}

Then in SpectreConsoleRenderedObject use

 public void Output(IDumpOutput output)
    {
        var console = AnsiConsole.Create(new AnsiConsoleSettings
        {
            Ansi = AnsiSupport.No,
            ColorSystem = ColorSystemSupport.Detect,
            //Out = new AnsiConsoleOutput(output.TextWriter),
            Out = new LargeConsoleSizeOutput(output.TextWriter),
        });

        console.Write(_renderable);
        console.WriteLine();
    }

I suppose the best way would be to always use a custom IAnsiConsoleOutput and let the user define if he wants to tweak the Width and Height through the RendererConfig.

MoaidHathot commented

@tcortega, we can grant the user this flexibility, but for some APIs, like DumpText, we should use larger width/height by default. Which is simple to do, we just need to auto-adjust the RendererConfig when DumpText is called, if the user did not provide a width/height explicitly.

tcortega commented

@tcortega, we can grant the user this flexibility, but for some APIs, like DumpText, we should use larger width/height by default. Which is simple to do, we just need to auto-adjust the RendererConfig when DumpText is called, if the user did not provide a width/height explicitly.

Regarding to the terminal size, I think that if we're going to have a default terminal size that is too large, it might not be worth keep giving an user the option to print the table as expanded.

It's probably best if we leave things the way they currently work, where Spectre.Console figures out the size of the terminal, but still give an user the choice to override the width whenever he needs it.

Also, I was debugging the problem reported on the issue I linked above, and it seems to be non-related at all to what we're facing. It is specific to the fact that the guy is using a Tree in his tables.

MoaidHathot commented

@tcortega, we won't change how Spectre.Console determines the size of the Terminal, but in DumpText there is no Terminal, so it makes since IMO to use a large default, since spectre.Console uses the smallest size when it can't detect Terminal Rendering, leading to this Issue.
I'm hesitant in keeping Expanded and NoWrap since they may lead to the issue I was describing before, which is when Spectre.Console decide not to render a component because there is not enough space, at least until we have a workaround for that, maybe by rendering a warning instead of the ... that Spectre.Console put there.

MoaidHathot commented

@tcortega, we won't change how Spectre.Console determines the size of the Terminal, but in DumpText there is no Terminal, so it makes since IMO to use a large default, since spectre.Console uses the smallest size when it can't detect Terminal Rendering, leading to this Issue. I'm hesitant in keeping Expanded and NoWrap since they may lead to the issue I was describing before, which is when Spectre.Console decide not to render a component because there is not enough space, at least until we have a workaround for that, maybe by rendering a warning instead of the ... that Spectre.Console put there.

This issue is fixed with the following commit by doing the following:

  • The dimensions of the rendered area (width/height) is now configurable
  • DumpText set a large default for the rendered area's dimensions (users can change it)
  • The rendered area dimensions' can be overridden by the user via OutputConfig

This fix will be part of this month's release.
Thanks @tcortega & @hub-stuff

MoaidHathot commented

@hub-stuff, this is now has been released in v0.6.0.
Thanks @tcortega

  • ❤️1
hub-stuff commented
MoaidHathot commented

@hub-stuff, sure. I've opened issue #15 to track it. I will look into it, planning to release it this month.