Expand tabs => configurable number of spaces to fix #150

[chris-laplante]

No description provided.

[djc]

Thinking about this some more, you replaced tabs at the line level, but most of the formatters can't actually emit tabs. Probably only from literals, message and prefix? So it might pay off to be precise about that.

[chris-laplante]

Thinking about this some more, you replaced tabs at the line level, but most of the formatters can't actually emit tabs. Probably only from literals, message and prefix? So it might pay off to be precise about that.

Yes, I agree. The code changes I am working on now will only replace inside those elements. Just wanted to check if you think it is acceptable complexity to also store away the original message & prefix and re-expand them when tab width changes?

[djc]

How about we only do that if message/prefix/template actually contained any tabs?

[chris-laplante]

How about we only do that if message/prefix/template actually contained any tabs?

So like this?

enum StringMaybeTabs {
    NoTabs(Cow<'static, str>),
    Tabs {
        original: Cow<'static, str>,
        expanded: String,
    }
}

[djc]

Something like that, yes. I'll need to see it in context.

[chris-laplante]

I've been working on this but it is turning out to be pretty ugly code. Trying to keep track of what strings are already expanded vs. not and then re-expanding everything when you change the tab size is unwieldly. To simplify things, what if I just added some mutually exclusive feature flags called "expand_tabs_four_spaces" and "expand_tabs_eight_spaces"? Realistically, I can't imagine why someone would change the size of a tab at runtime anyway. And if someone wants to do it, they can submit the PR :).

[djc]

I wonder if we can query the terminal for its tab size?

https://unix.stackexchange.com/questions/46368/set-tab-width-in-gui-terminal

[chris-laplante]

I wonder if we can query the terminal for its tab size?

https://unix.stackexchange.com/questions/46368/set-tab-width-in-gui-terminal

I looked into that but I don't think there's any way to query it unless you actually print a tab and measure the difference in cursor position (e.g. https://unix.stackexchange.com/a/389266). Plus, who knows how Windows / macOS work.

[djc]

Maybe open an issue against console? This seems like the kind of thing that it could provide.

[chris-laplante]

Maybe open an issue against console? This seems like the kind of thing that it could provide.

OK, will do.

[chris-laplante]

On further thought, I don't think that console are any better positioned to fix this than we are, so I'm going to push on and attempt to make a fix.

[chris-laplante]

OK, here's my latest attempt at fixing #150. Still need to write tests for it.

[djc]

Sorry, it's been a very busy week, but this is definitely still on my list.

[djc]

In #420 this is changed to pass in a Formatter directly. How do we change tab expansion in that case?

[chris-laplante]

I think we'll have to modify this line to apply the tab expansion on buf before handing over to the formatter: https://github.com/console-rs/indicatif/pull/420/files#diff-5452dc5fb15e73154c0ecc33e87a14379a97f817fe0cdb0fb9d17df712feb8fbR243

[djc]

Approved with the remaining comments addressed. This has turned out quite nice!

[chris-laplante]

OK, just going to wait for the tests to complete then I'll merge :)