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

enable a way to elide the message "header" #167

Open
BurntSushi opened this issue Dec 20, 2024 · 2 comments · May be fixed by #171
Open

enable a way to elide the message "header" #167

BurntSushi opened this issue Dec 20, 2024 · 2 comments · May be fixed by #171

Comments

@BurntSushi
Copy link
Member

I'm somewhat new to this crate, so apologies for any category or vernacular errors that I make.

I'm looking at migrating ruff from annotate-snippets 0.9 to annotate-snippets 0.11. But I'm hitting an issue where we would like to use our own headers for snippets. This worked in 0.9, but I can't find a way to replicate it in 0.11. To make this concrete, consider this program using 0.9:

use annotate_snippets::{
    display_list::{DisplayList, FormatOptions},
    snippet::{Annotation, AnnotationType, Slice, Snippet, SourceAnnotation},
};

fn main() {
    let snippet = Snippet {
        title: None,
        footer: vec![
            Annotation {
                id: None,
                label: Some(
                    "Replace with `None`; initialize within function",
                ),
                annotation_type: AnnotationType::Help,
            },
        ],
        slices: vec![
            Slice {
                source: "# Docstring followed by a newline\n\ndef foobar(foor, bar={}):    \n    \"\"\"\n    \"\"\"\n",
                line_start: 1,
                origin: None,
                annotations: vec![
                    SourceAnnotation {
                        range: (
                            56,
                            58,
                        ),
                        label: "B006",
                        annotation_type: AnnotationType::Error,
                    },
                ],
                fold: false,
            },
        ],
        opt: FormatOptions {
            color: false,
            anonymized_line_numbers: false,
            margin: None,
        },
    };
    println!("{message}", message = DisplayList::from(snippet));
}

It has this output:

  |
1 | # Docstring followed by a newline
2 |
3 | def foobar(foor, bar={}):
  |                      ^^ B006
4 |     """
5 |     """
  |
  = help: Replace with `None`; initialize within function

In ruff, this gets transformed slightly to include a header:

B006_1.py:3:22: B006 [*] Do not use mutable data structures for argument defaults
  |
1 | # Docstring followed by a newline
2 |
3 | def foobar(foor, bar={}):
  |                      ^^ B006
4 |     """
5 |     """
  |
  = help: Replace with `None`; initialize within function

Now consider this program, using 0.11, which tries to mimic the above program:

use annotate_snippets::{Level, Renderer, Snippet};

fn main() {
    let source = "# Docstring followed by a newline\n\ndef foobar(foor, bar={}):    \n    \"\"\"\n    \"\"\"\n";
    let src_annotation = Level::Error.span(56..58).label("B006");
    let snippet = Snippet::source(source)
        .line_start(1)
        .annotation(src_annotation)
        .fold(false);
    let footer =
        Level::Help.title("Replace with `None`; initialize within function");
    let message =
        Level::Error.title("<TITLE>").snippet(snippet).footer(footer);
    println!("{}", Renderer::plain().render(message));
}

It has this output:

error: <TITLE>
  |
1 | # Docstring followed by a newline
2 |
3 | def foobar(foor, bar={}):
  |                      ^^ B006
4 |     """
5 |     """
  |
  = help: Replace with `None`; initialize within function

Which is... very close to what we had before. But it now has an error: <TITLE> line that I don't think can be removed. At least, it seems to correspond to a requirement to create a message by providing a Level with a title.

Is this an intended change to the library that isn't meant to be configurable? If so, I think our options are to either go our own way or to explore changing the formatting of our own diagnostics (which we could do, but I definitely want to see if I can de-couple upgrading annotate-snippets from "let's reconsider our diagnostic formatting").

I do see some other issues and PRs seemingly related to this issue, but I wasn't 100% sure if this was a strict duplicate or not. So I wanted to put my use case in writing and see where that takes us.

@epage
Copy link
Contributor

epage commented Dec 20, 2024

The change was made in #91 that didn't include the design decisions behind the API. I think they are in a hackmd document that @Muscraft has.

With the old API, there wasn't clear reasoning for why choices were made and our primary audience atm is cargo and rustc, so we are making decisions in that direction, including wanting to make sure things are done "correctly". Note this doesn't preclude supporting other use cases.

We make the header out of (level, code, title) while you need a custom header. Would it work to pass in your custom header to annotate-snippets or do you need the two to be rendered separately?

@BurntSushi
Copy link
Member Author

Yeah I'm still evaluating whether we want to own our own rendering code or not, but at least for now, yes, I think we could make it work if annotate-snippets allowed passing a custom header.

BurntSushi added a commit to astral-sh/ruff that referenced this issue Dec 20, 2024
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 8, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 8, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 8, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to BurntSushi/annotate-snippets-rs that referenced this issue Jan 8, 2025
When combined with an empty `title`, this results in the header being
omitted entirely.

This isn't what @epage suggested in rust-lang#167 since changing our rendering to
pass in a custom header is a bigger refactor than what I've been able to
get to. The goal of this change was to be very small without requiring
bigger code changes on our end. Unfortunately, this is a breaking
change, so I'm not sure what the appetite is for this.

Fixes rust-lang#167
@BurntSushi BurntSushi linked a pull request Jan 8, 2025 that will close this issue
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 9, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 10, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 13, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 14, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 14, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 15, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
BurntSushi added a commit to astral-sh/ruff that referenced this issue Jan 15, 2025
This is a tiny change that, perhaps slightly shady, permits us to use
the `annotate-snippets` renderer without its mandatory header (which
wasn't there in `annotate-snippets 0.9`). Specifically, we can now do
this:

    Level::None.title("")

The combination of a "none" level and an empty label results in the
`annotate-snippets` header being skipped entirely. (Not even an empty
line is written.)

This is maybe not the right API for upstream `annotate-snippets`, but
it's very easy for us to do and unblocks the upgrade (albeit relying on
a vendored copy).

Ref rust-lang/annotate-snippets-rs#167
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 a pull request may close this issue.

2 participants