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

Weird tab character rendering #73

Closed
jlkiri opened this issue Sep 29, 2021 · 4 comments · Fixed by #82
Closed

Weird tab character rendering #73

jlkiri opened this issue Sep 29, 2021 · 4 comments · Fixed by #82
Labels
bug Something isn't working Hacktoberfest Stuff that's good for hacktoberfest contribs help wanted Extra attention is needed

Comments

@jlkiri
Copy link
Contributor

jlkiri commented Sep 29, 2021

I'm using miette in a parser that deals with tab \t indents and so far noticed the following:

  • miette renders one \t as the number of spaces configured in the terminal (OK)
  • miette counts one \t's width as equal to one space and displays the arrow/label according to this width (Breaks because of the above, but should be possible to deal with by the developer)

So I decided to calculate number of tabs in the line, multiply it by tab width, and add that to the offset minus the number of tabs. Example:

  1. \t\t with tab width 4 = 8
  2. Offset of some char that caused the error is 10
  3. We add 10 + 8 and subtract 2 (number of tabs) and give that number as SourceSpan (16, 0)

In my repro this places the arrow in the correct position. But weird stuff happens if I have a really big offset (exactly 37 and more chars). It either just renders the line without rendering arrow/label or simply panics with this error:

thread 'main' panicked at 'failed printing to stderr: formatter error', library/std/src/io/stdio.rs:935:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Here's a reproduction:
https://github.com/jlkiri/miette-tab-error-repro

@zkat zkat added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Sep 30, 2021
@zkat
Copy link
Owner

zkat commented Sep 30, 2021

sigh. I guess we have to use unicode-width after all.

@jlkiri can I interest you in filing a PR to add this to the graphical renderer?

@zkat zkat removed the good first issue Good for newcomers label Sep 30, 2021
@jlkiri
Copy link
Contributor Author

jlkiri commented Sep 30, 2021

Ok, I'll take a look at the code! (Btw I temporarily deal with this by simply replacing tabs with spaces before rendering)

@jlkiri
Copy link
Contributor Author

jlkiri commented Oct 1, 2021

So it turned up to be more complicated than I expected. I figured out that render_single_line_highlights is probably the best place to calculate stuff but I'm not sure how unicode-width fits there because it counts width of control chars as zero.

Anyway while I was doing this I thought that it might be best to have an option replace_tabs that is Option<usize> that replaces \t with usize spaces. What do you think?

@zkat
Copy link
Owner

zkat commented Oct 1, 2021

Oh I love that. Maybe call it tab_width instead tho?

@zkat zkat added the Hacktoberfest Stuff that's good for hacktoberfest contribs label Oct 1, 2021
@zkat zkat closed this as completed in #82 Oct 4, 2021
zkat pushed a commit that referenced this issue Oct 6, 2021
zkat pushed a commit that referenced this issue Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Hacktoberfest Stuff that's good for hacktoberfest contribs help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants