Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Obscure width bug #130

Open
cafkafk opened this issue Aug 13, 2023 · 8 comments
Open

bug: Obscure width bug #130

cafkafk opened this issue Aug 13, 2023 · 8 comments
Labels
errors › invalid output good first issue Good for newcomers help wanted Extra attention is needed

Comments

@cafkafk
Copy link
Member

cafkafk commented Aug 13, 2023

I'm adding this just so I don't forget about it.

Seems to occur around COL 399
2023-08-13_06-37

Seems not to occur before COL 398
2023-08-13_06-37_1

I don't have time to look more into this right now, and this probably will not matter in this decade, but it's still a bug.

@cafkafk cafkafk added help wanted Extra attention is needed good first issue Good for newcomers errors › invalid output labels Aug 13, 2023
@gierens
Copy link
Member

gierens commented Aug 13, 2023

It gets even weirder:
Screenshot from 2023-08-13 15-12-59

@PThorpe92
Copy link
Member

So my initial thoughts are This
might very well fix (or I guess could even complicate) this and #129 ?

Not that it doesn't seem worth looking into... but @gierens I assume you are taking a look at the width issues?

@gierens
Copy link
Member

gierens commented Aug 13, 2023

So I looked into this, and traced it back to this: https://github.com/eza-community/eza/blob/main/src/output/grid_details.rs#L167-L177 ... Here it tries to find the largest number of columns where the grid still fits in the requested width.

It starts at 2 columns and ramps it up as long as the "supposed" width of the displayed grid is smaller than the console width. I say "supposed" because I'm pretty sure the values reported back by d.width() are off. In case of my screenshot above the loop ends up at 11 grid columns "supposedly" fitting into 482 terminal columns, which they obviously don't.

Since this d.width() is a term_grid function this is probably due to the one-off bug in it (ogham/rust-term-grid#15) referenced in the issue you mentioned. So I would agree that moving to the patched fork should probably fix this.

@gierens
Copy link
Member

gierens commented Aug 13, 2023

I also think that #129 is probably unrelated to this term-grid bug, but I will write more detailed comment about it in that issue.

@PThorpe92
Copy link
Member

At the very least, I think substantial effort would be better spent on/after migration to uutils. I am surprised it's not on crates.io as it looks like it's the only one they haven't published.

@gierens
Copy link
Member

gierens commented Aug 13, 2023

I also think that #129 is probably unrelated to this term-grid bug, but I will write more detailed comment about it in that issue.

Confirmed, #129 is unrelated, just created a PR for that one: #136

@cafkafk cafkafk changed the title Obscure width bug bug: Obscure width bug Aug 28, 2023
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions
Copy link

This issue was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2023
@cafkafk cafkafk reopened this Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors › invalid output good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants