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

fix(render): On fold, strip prefix/suffix rather than fold #109

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented Mar 21, 2024

As we don't show ... on non-first lines without fold, it seems to make
sense to do the same thing for fold.
To show this, we likely would want to add more nuance than just fold.

This has the benefit of offering a "magic mode" where the annotated
lines get selected automatically.

This was implemented by pre-processing things which should keep overhead
low as other calculations down the line aren't done.

As `fold` is special machinery, I made that test under that.

The others, I couldn't tell what different cases they were covering, so
I kept the names.
I did name them with the `ann` prefix to highlight these are testing
annotations.
As we don't show `...` on non-first lines without fold, it seems to make
sense to do the same thing for fold.
To show this, we likely would want to add more nuance than just `fold`.

This has the benefit of offering a "magic mode" where the annotated
lines get selected automatically.

This was implemented by pre-processing things which should keep overhead
low as other calculations down the line aren't done.
Copy link
Member

@Muscraft Muscraft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good change overall, but we will want to add the ability for a user to specify the number of lines to keep.

@Muscraft Muscraft merged commit 65b6bef into rust-lang:master Mar 21, 2024
12 of 13 checks passed
@epage epage deleted the fold branch March 21, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants