console-rs / indicatif

A command line progress reporting library for Rust
MIT License
4.36k stars 241 forks source link

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

Closed chris-laplante closed 2 years ago

djc commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

chris-laplante commented 2 years ago

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 commented 2 years ago

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

chris-laplante commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

chris-laplante commented 2 years ago

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

OK, will do.

chris-laplante commented 2 years ago

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 commented 2 years ago

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

djc commented 2 years ago

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

chris-laplante commented 2 years ago

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