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

docs: Fix and add docs for Level #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

icecream17
Copy link

Have the documentation for Level reflect actual appearance.


The output from this file:

(43 lines)
//! Provides a user-friendly error if the given file has the string `tab`
//!
//! A silly utility to test annotate_snippets (and anstream)

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

#[derive(Parser)]
struct Cli {
    file_path: std::path::PathBuf,
}

fn main() {
    let Cli { file_path } = Cli::parse();
    let path_string = file_path.to_string_lossy().to_string();

    // Bad error message; don't do this!
    let contents =
        std::fs::read_to_string(file_path).expect("Should have been able to read the file");

    let renderer = Renderer::styled();
    let message = if let Some(index) = contents.find("tab") {
        Level::Error.title("no tabs allowed!").snippet(
            Snippet::source(&contents)
                .line_start(1)
                .origin(&path_string)
                .fold(true)
                .annotation(
                    Level::Error
                        .span(index..(index + 3))
                        .label("Found `tab` here"),
                )
                .annotation(Level::Warning.span(index..(index + 3)).label("Warning"))
                .annotation(Level::Info.span(index..(index + 3)).label("Info"))
                .annotation(Level::Note.span(index..(index + 3)).label("Note"))
                .annotation(Level::Help.span(index..(index + 3)).label("remove `tab`")),
        )
    } else {
        Level::Info.title("No \"tabs\" found, congrats!")
    };
    anstream::println!("{}", renderer.render(message));
}

Results in this output:
image

@icecream17 icecream17 changed the title docs: fix and add docs for Level enum docs: Fix and add docs for Level Aug 19, 2024
@icecream17
Copy link
Author

Note: this is potentially inconsistent with cargo and clippy
image
image

@icecream17
Copy link
Author

Note: This is cyan rather than blue:
(found by using manual output while trying to work around #101:)
image

If a reviewer comments, I will change the wording to "cyan"

@Muscraft
Copy link
Member

Thank you for wanting to make them consistent. However, I think the original doc comments mentioning the color and character was not the right call. This is primarily due to three reasons:

  1. Colors are configurable by end users
  2. Colors are platform dependent
  3. Characters being tied to a Level is for historical reasons and will be removed in the future when we match rustc's idea of Primary and Secondary for Annotations

I do think it is a good idea to document the Level text and default color, but graphically and in a way that won't go stale. This can be done by utilizing fixture tests and including the SVGs as doc comments. The rough process would be:

  • Add a new docs folder inside of tests/fixtures for the new tests
  • Add a basic example for each Level
  • Link to the newly created SVGs in the same way as we do in the lib docs

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