MoaidHathot / Dumpify

Adding `.Dump()` extension methods to Console Applications, similar to LinqPad's.
MIT License
959 stars 40 forks source link

Question: Is there a way to disable line wrapping? #3

Closed hub-stuff closed 1 year ago

hub-stuff commented 1 year ago

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 1 year ago

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 1 year ago

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 1 year ago

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

MoaidHathot commented 1 year ago

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

tcortega commented 1 year ago

Unfortunately, this change alone is not enough. https://github.com/MoaidHathot/Dumpify/issues/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 https://github.com/MoaidHathot/Dumpify/issues/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: https://github.com/spectreconsole/spectre.console/issues/1185

MoaidHathot commented 1 year ago

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 1 year ago

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 1 year ago

@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 1 year ago

@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 1 year ago

@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 1 year ago

@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:

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

MoaidHathot commented 1 year ago

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

hub-stuff commented 1 year ago

Could you please sign dumpify? One odmy projects requiee this.

wt., 18 lip 2023, 02:08 użytkownik Moaid Hathot @.***> napisał:

@hub-stuff https://github.com/hub-stuff, this is now has been released in v0.6.0 https://github.com/MoaidHathot/Dumpify/releases/tag/0.6.0. Thanks @tcortega https://github.com/tcortega

— Reply to this email directly, view it on GitHub https://github.com/MoaidHathot/Dumpify/issues/3#issuecomment-1639061125, or unsubscribe https://github.com/notifications/unsubscribe-auth/BAHDQ37SMXHDPXDREIC42F3XQXHY3ANCNFSM6AAAAAAYVWG7IU . You are receiving this because you were mentioned.Message ID: @.***>

MoaidHathot commented 1 year ago

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