attempt to stabilize annotate snippet #150032
Conversation
rustbot
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
annotate-snippets-stable
branch
from
e4ad760 to
0298975
Compare
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
annotate-snippets-stable
branch
from
0298975 to
9932c0b
Compare
There was a problem hiding this comment.
the main problem I faced is that there is no difference on an tests output? and I don't know if there should be any difference, so yeah, maybe I missed something or maybe it should be like this
There really shouldn't be any test difference as annotate-snippets is already used on nightly, which means that all tests are currently ran against it.
| // FIXME: This hack should be removed once annotate_snippets is the | ||
| // default emitter. | ||
| if suggestions_expected > 0 && report.is_empty() { | ||
| group = group.element(Padding); | ||
| } | ||
|
|
There was a problem hiding this comment.
I would move this change (and anything related) to its own commit after the change to make annotate-snippets the default. This will make it so that the switch to annotate-snippets shows now visible test changes (I think).
| let format = match format { | ||
| ErrorOutputType::Json { pretty: true, .. } => "pretty-json", | ||
| ErrorOutputType::HumanReadable { kind, .. } => match kind { | ||
| HumanReadableErrorType::AnnotateSnippet { unicode: false, .. } => "human-annotate-rs", |
There was a problem hiding this comment.
I would make the removal of the human-annotate-rs flag its own commit, although I am not sure if it should be before or after the commit switching to annotate-snippets as the default.
Note: That change will also require removing any references to it
| ErrorOutputType::Json { pretty: true, .. } => "pretty-json", | ||
| ErrorOutputType::HumanReadable { kind, .. } => match kind { | ||
| HumanReadableErrorType::AnnotateSnippet { unicode: false, .. } => "human-annotate-rs", | ||
| HumanReadableErrorType::AnnotateSnippet { unicode: true, .. } => "human-unicode", |
There was a problem hiding this comment.
This shouldn't remove support for human-unicode as is being tracked separately from using annotate-snippets as the default emitter.
| #[default] | ||
| HumanReadable { | ||
| kind: HumanReadableErrorType = HumanReadableErrorType::Default { short: false }, | ||
| kind: HumanReadableErrorType = HumanReadableErrorType::AnnotateSnippet { short: false, unicode: false }, |
There was a problem hiding this comment.
I wonder if it would make sense to remove HumanReadableErrorType::Default as a variant of HumanReadableErrorType. This would make it easier to verify that everything is truly using HumanReadableErrorType::AnnotateSnippet.
4 participants
not for merge, at least for now
part of #149932
not sure how correct this approach is, also it maybe will fail on a cargo tests because I had some difficult time on a blessing them
so this PR is just to show to Scott @Muscraft on what I've done already and if I'm on a right track
the main problem I faced is that there is no difference on an tests output? and I don't know if there should be any difference, so yeah, maybe I missed something or maybe it should be like this