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

Combine Linter rules and Diagnostic messages pages? #4498

Open
MaryaBelanger opened this issue Jan 9, 2023 · 12 comments
Open

Combine Linter rules and Diagnostic messages pages? #4498

MaryaBelanger opened this issue Jan 9, 2023 · 12 comments
Assignees
Labels
e2-days Can complete in < 5 days of normal, not dedicated, work from.team Reported by Dash docs team member p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. st.triage.ltw Indicates Lead Tech Writer has triaged t.cli-tools Relates to the dart command line tools t.diagnostics Relates to diagnostics, analysis, or linting of code

Comments

@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Jan 9, 2023

This is part of the greater effort to restructure and improve the diagnostic messages documentation.

Incorporating the linter rules page into the diagnostic messages page seems like a good idea because lints are a type of diagnostic message (along with errors and warnings), so it makes sense conceptually join them instead of having a separate, slightly different, standalone page.

  • Keeps all the diagnostics messages in one place, easy to ctrl+f search
  • Reduces page clutter. Especially important as we plan to restructure dart.dev
  • Creates a cognitive connection between lints and other diagnostics

BUT, the potential length of the page is a concern, so we haven't finalized whether to do this.

  • Load time could become a problem.
    • The number of messages on the diagnostic messages page is already very high.
    • There are many already-existing diagnostics that still need to be added.
    • There will always be more created as features develop.
    • Adding the linter messages to that page, that are built from a different source, that will also always continually be added to over time, exacerbates that.
  • Scroll length is not the issue.
    • Both pages are only used as references, where users will likely only ever be linked to the specific message they're concerned with.

@mit-mit, @bwilkerson @pq and I talked about combining the pages, but are concerned with the page length becoming an issue. We wanted to check with you before deciding one way or another, benefit vs. cost, etc.

@mit-mit
Copy link
Member

mit-mit commented Jan 9, 2023

We could use a detail page for each lint -- like on https://dart-lang.github.io/linter/lints/ -- and then just have index pages that list them all?

@bwilkerson
Copy link
Member

We could use a detail page for each lint ...

In terms of content, the detail pages for lints are effectively equivalent to the individual diagnostic message docs on https://dart.dev/tools/diagnostic-messages. If each lint's detail page is on a separate page, then we should consider putting each non-lint diagnostic's detail page on a separate page, though as was pointed out above, that could cause page clutter.

... and then just have index pages that list them all?

I think we need a page that does more than just list them all, I think we need a page (or multiple pages?) that guides users through the process of configuring lints for their package. One possibility that I could imagine is to have the lints (or maybe only those that aren't in either 'core' or 'recommended' grouped in some meaningful way with a brief description of why a user might want to enable it (a brief sales pitch, if you will).

@parlough parlough added p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. t.cli-tools Relates to the dart command line tools e2-days Can complete in < 5 days of normal, not dedicated, work analysis labels Jan 9, 2023
@parlough
Copy link
Member

parlough commented Jan 9, 2023

Sounds to me an ideal solution is to split out the diagnostic information of lints from the why. People visiting the diagnostic information from tooling/IDEs already chose to enable the lint and don't need the sales pitch or information about using it (version, incompatible lints, etc). They just need information in a way that is consistent with the other diagnostics.

For those users who are choosing new lints, they need a page with all lints (categorized) like the old index, the "brief sales pitch" Brian talked about. On this page, details about the sets they are in, release version, incompatible lints, and state can be included through easy to visually parse labels or icons. Links to Effective Dart could also be here as necessary. We could also have a button to easily copy the name for adding to the yaml file.

@parlough
Copy link
Member

parlough commented Jan 9, 2023

Since it sounds like it is expected to mostly only visit diagnostic messages from tooling and IDEs, perhaps it doesn't make sense to have such a long page. We're already having users leave their workflow to visit a site and serving a bunch of information when they likely won't be manually scrolling for other diagnostics doesn't make a lot of sense.

I'm not too concerned about page clutter, especially since we could generate the diagnostic pages as part of the build process from some YAML or JSON structure. Perhaps we could even serve Markdown files too so IDEs could surface the documentation inline/in a popup (are they capable of that or is that even valuable @bwilkerson?).

@bwilkerson
Copy link
Member

Sounds to me an ideal solution is to split out the diagnostic information of lints from the why.

I really like that idea.

... we could generate the diagnostic pages as part of the build process from some YAML or JSON structure.

As you know, we're already building the diagnostic messages page from a YAML file. I'd kind of like to have a separate YAML file for the lints, but there's some value in leaving the docs in the individual .dart files with the implementation so we might go that route, but the page content will still be generated either way.

Perhaps we could even serve Markdown files too so IDEs could surface the documentation inline ...

I'm not aware of such support in LSP, but I'm not as familiar with LSP as I need to be at this point, so this is a better question for @DanTup .

@DanTup
Copy link
Contributor

DanTup commented Jan 9, 2023

Perhaps we could even serve Markdown files too so IDEs could surface the documentation inline (are they capable of that or is that even valuable @bwilkerson?).

I'm not exactly sure where we're referring to with "inline" here. We can render a limited set of markdown in some places in the IDE (for example we could show rendered markdown in the code completion details of the lint names when completing in pubspec.yaml, or when hovering over the lint names). Is that the sort of thing you had in mind?

As for diagnostics (the things shown in the Problems list in VS Code), markdown is not currently supported there. There's an open issue at microsoft/vscode#54272 (it would need to be supported in both VS Code and LSP for us to have it show up there).

@parlough
Copy link
Member

parlough commented Jan 9, 2023

I was thinking more so when a diagnostic is reported and you hover rather than information on the lints themselves. If diagnostics don't support Markdown yet, it's not something we really need to consider now. Thanks for the extra context!

Even without that functionality, I still see value in splitting the diagnostic information into multiple pages. We can always explore serving Markdown later if that becomes relevant.

@DanTup
Copy link
Contributor

DanTup commented Jan 9, 2023

FWIW, one thing we can do today (which you may know about, but it's not super obvious in VS Code's UI), is attach a URL to an error code so that error code (in the problems view) can be Ctrl+Clicked. I think we already use this for some diagnostics (perhaps the ones listed at https://dart.dev/tools/diagnostic-messages), but we could do the same for lints (if we don't already).

@bwilkerson
Copy link
Member

... we could do the same for lints (if we don't already).

We do.

@atsansone
Copy link
Contributor

@DanTup , @bwilkerson : How much of this could we automate? Maybe not individual pages (if we go in that direction), but in terms of a list or "index"? This could let Docs know that something has changed because of a broken link or something similar.

@parlough
Copy link
Member

parlough commented Jan 31, 2023

Automating what part @atsansone? If we go in the route of individual pages, we'll be able to automate their generation from YAML file(s) using Jekyll's data/generator support or a Dart script if that's preferred, the same goes for any index page based off the same data.

As for keeping the site up to date with those sources, we could setup a GitHub action workflow which incorporates the YAML source(s) as necessary through a PR.

The only manual process would be writing the new lint diagnostic documentation itself.

@bwilkerson
Copy link
Member

Agreed. I would expect that we could automate the whole process, other that writing and reviewing the docs.

@atsansone atsansone added the t.diagnostics Relates to diagnostics, analysis, or linting of code label Apr 5, 2023
@atsansone atsansone added the st.triage.ltw Indicates Lead Tech Writer has triaged label May 9, 2023
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jun 2, 2023
This allows more flexibility as we can configure the redirect overtime, compared to the GitHub hosted site which we have less control over. It could even redirect to the old linter site in the meantime if desired.

Contributes to https://github.com/dart-lang/linter/issues/4411 and dart-lang/site-www#4498

Change-Id: I3512002cdf7f62c0338d67cec0d7091f1166479d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307000
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
parlough added a commit that referenced this issue Jun 27, 2023
This PR is meant to finalize work to make dart.dev a satisfactory
replacement for
[`dart-lang.github.io/linter/`](https://dart-lang.github.io/linter/)
while `dart-lang/linter` moves to the SDK and we work on consolidating
and improving documentation
(#4498).

It does this by (staged links after colon (`:`)):

- Updating the information on dart.dev/lints for the latest state of the
linter:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Making dart.dev/lints an index page rather than including all
information:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Moving the long-form lint rule documentation to individual pages found
at `dart.dev/lints/<rule>`:
[/lints/avoid_dynamic_calls](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/avoid_dynamic_calls)
- It does this by using a Jekyll page generator based on
`linter_rules.json`.
- Adding a textual reference pointing to the recently released all
linter rules page
(38973de)
found at:
[/lints/all](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/all)
- Updating and links and references for these previous changes (mostly
Effective Dart).

Contributes to https://github.com/dart-lang/linter/issues/4460,
#4498,
#4499

**Note:** This is an intermediate step and doesn't aim to improve the
formatting or style of the information on the index or individual pages.
That will be part of follow-up work as we work on improvements to the
diagnostic linter rule documentation.
@atsansone atsansone added the from.team Reported by Dash docs team member label Aug 8, 2023
rmacnak-google pushed a commit to rmacnak-google/site-www that referenced this issue Sep 5, 2023
This PR is meant to finalize work to make dart.dev a satisfactory
replacement for
[`dart-lang.github.io/linter/`](https://dart-lang.github.io/linter/)
while `dart-lang/linter` moves to the SDK and we work on consolidating
and improving documentation
(dart-lang#4498).

It does this by (staged links after colon (`:`)):

- Updating the information on dart.dev/lints for the latest state of the
linter:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Making dart.dev/lints an index page rather than including all
information:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Moving the long-form lint rule documentation to individual pages found
at `dart.dev/lints/<rule>`:
[/lints/avoid_dynamic_calls](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/avoid_dynamic_calls)
- It does this by using a Jekyll page generator based on
`linter_rules.json`.
- Adding a textual reference pointing to the recently released all
linter rules page
(dart-lang@38973de)
found at:
[/lints/all](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/all)
- Updating and links and references for these previous changes (mostly
Effective Dart).

Contributes to https://github.com/dart-lang/linter/issues/4460,
dart-lang#4498,
dart-lang#4499

**Note:** This is an intermediate step and doesn't aim to improve the
formatting or style of the information on the index or individual pages.
That will be part of follow-up work as we work on improvements to the
diagnostic linter rule documentation.
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jun 17, 2024
Dependent on dart-lang/site-www#5914 landing first.

Bug: dart-lang/site-www#4498
Change-Id: I378debd5d484e17f18d59c49173a5d010f05b6df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371785
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Sam Rawlins <srawlins@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2-days Can complete in < 5 days of normal, not dedicated, work from.team Reported by Dash docs team member p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. st.triage.ltw Indicates Lead Tech Writer has triaged t.cli-tools Relates to the dart command line tools t.diagnostics Relates to diagnostics, analysis, or linting of code
Projects
None yet
Development

No branches or pull requests

6 participants