-
Notifications
You must be signed in to change notification settings - Fork 14.1k
fix(parse): Limit frontmatter fences to 255 dashes #149358
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
Conversation
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
This comment has been minimized.
This comment has been minimized.
|
Looks good, but... This error message seems a bit verbose, maybe we shouldn't show all this "-"'s or something, like, just say something "hey, this line is a bit longer than it should be, can you fix it" |
|
For raw string literals, we show I'm assuming this isn't being collapsed down because it is multi-line (@Muscraft maybe a potential future change for annotate-snippets). Maybe if I switch the span to just the opening, it will look better. |
|
I tried only showing the one line and it doesn't get snipped in the middle like Is there a way for me to have the span without it getting rendered in my error or are you recommending I remove the span completely? |
Honestly I don't have exact or better solution that I can suggest you here, just wondering about how can we make it less verbose Can you show output without span completely to just see how it looks |
|
I just pushed I'll now update it to no span |
|
upd to prev message: I like the output from But again, I can't help you with implementing or something since I don't know much about this in particular |
|
Without a span not pushed yet as I'm looking how to handle this in the ui test |
|
Found it, |
|
I personally think that no span is fine in case we couldn't adopt approach from I may take a look tomorrow on how it was made in there for hashes, it's really looks cool I also have a conjecture about how it was made (but it's just a wild guess): it was just hard coded - suggestion always have 38 hashes on right side and 58 on left side despite amount of actual hashes, the only thing is depends on amount of hashes is last part in error message But still, it looks like an actual span, maybe some magic with BytePos... Yeah I will take a look on implementation for this just out of curiosity |
|
the error emitter / annotate-snippets has logic to shrink code snippets to fit on a screen. I'm unsure why it isn't activating in this case. I had wondered if there is an issue with the emitter assuming an infinite screen size for tests but I didn't see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - this is an pretty niche error so I don't think it needs to be perfectly worded and we can always improve it in future
|
@bors r+ rollup |
Rollup of 9 pull requests Successful merges: - #148690 (Implement `clamp_magnitude` method for primitive floats & signed integers) - #149102 (stabilize maybe_uninit_slice) - #149269 (cmse: do not calculate the layout of a type with infer types) - #149299 (Fudge infer vars in the cause code of `Obligation` intentionally) - #149344 (Don't suggest unwrap for Result in const) - #149358 (fix(parse): Limit frontmatter fences to 255 dashes ) - #149445 (make assoc fn inherit const stability from inherent `const impl` blocks) - #149479 (Fix indent in E0591.md) - #149496 (Fix #148889: Add label rib when visiting delegation body) r? `@ghost` `@rustbot` modify labels: rollup
Port rust-lang/rust#149358 over to Cargo
Port rust-lang/rust#149358 over to Cargo
### What does this PR try to resolve? Port rust-lang/rust#149358 over to Cargo ### How to test and review this PR?
Like raw string literals. As discussed on #148051.
Part of #136889