User hints

[thomas-zahner]

Sorry if this isn't the right place to discuss this, but I always found the message (this depends on your "accept" configuration) on every line a bit noisy. Maybe we should put it at the end, as we do with the GitHub token hint.

[stdin]:
     [301] https://mock.httpstatus.io/301 | Rejected status code: 301 Moved Permanently: Redirects may have been limited by "max-redirects".
     [404] https://mock.httpstatus.io/404 | Rejected status code: 404 Not Found
     
Hint: You can configure accepted response codes with the "accept" option".

We could introduce a Hint struct that collects all of those hints (GitHub token, accept codes, redirects detected).

Originally posted by @mre in #2012 (comment)

[mre]

I wonder if we should move it out of formatters. Maybe src/hints.rs?

[mre]

This limits the design space. I was thinking of a generic Vec of hints, as in

type Hints = Vec<Hint>;

Then we could just "push" hints into that and impl a nice Display for it. Done. 😉

[thomas-zahner]

Thank you for the feedback. For now I just kept the changes minimal and did the extraction, which is also why the PR is a draft.

Thinking about it I find it quite difficult to generalise all hints. It feels a bit like over-engineering, but maybe there is a good solution after all?

What would Hint be then?

Does it have state? Or is it just the rules and functions to know if the given hint should be triggered and if so, what the specific message was? Hints are only optionally triggered/shown and could take very different arguments.

The following wouldn't seem to work well.

type Hint = Box<dyn Display>;

type Hints = Vec<Hint>;

Instead a hint probably should define the rules to calculate if it should be triggered and what the message should be.

trait Hint {
    fn show() -> Option<String>;
}

But the above would still not work, because each hint would have different arguments. This meant that the Hint itself would have to contain state, but that wouldn't make sense either, I think.

[mre]

What would Hint be then?

Looking at the current usage pattern, Hint could just be a newtype wrapper around String?

#[derive(Debug)]
struct Hint(String);

impl Hint {
  fn show(&self) -> String {
    self.0
  }
}

Not sure if we need anything more than that? 🤔
It would be great to check that a hint is non-empty on construction, hence the newtype; but apart from that, I'd keep it pretty lightweight.

It could be that I'm not accounting for additional requirements, though.

[mre]

I would move this function and the other functions below out of the hints module. So the logic becomes: "If we detect an issue throughout the link checking, we can easily add a hint."

[thomas-zahner]

So would you "generate" the hint message at the location where we encounter it? I currently feel like moving this logic into the hint module makes sense.

[mre]

Good point. My idea was that we could provide a method like

fn add_hint<T: Into<Hint>>(hint: T);

And support converting strings to hints. So impl From<String> for Hint.
Not sure if that's a good idea, but that's what I had in mind to decouple the responsibilities.