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

Print rule's documentation links #186

Merged
merged 11 commits into from
Mar 22, 2022
Merged

Print rule's documentation links #186

merged 11 commits into from
Mar 22, 2022

Conversation

mizdra
Copy link
Owner

@mizdra mizdra commented Feb 24, 2022

fix: #89

blocked by: chalk/supports-color#97, gajus/table#113

Try it out

$ yarn run dev

Before

image

After

image

@mizdra mizdra added Type: Feature New Feature Status: Blocked Progress on the issue is Blocked help wanted Extra attention is needed labels Feb 24, 2022
@mizdra
Copy link
Owner Author

mizdra commented Feb 24, 2022

  • terminal-link uses supports-hyperlinks to determine if a terminal that supports hyperlinks is in use.
  • If the terminal does not support hyperlinks, it will fallback to not print the link.
  • It seems that there is a bug in supports-hyperlinks that causes it to determine that a terminal does not support hyperlinks, even if it does support them.
  • This is probably because supports-hyperlinks is running on a worker thread.
  • We need to report the problem to supports-hyperlinks.

@mizdra mizdra changed the title Revive documentation links Revive rule's documentation links Feb 24, 2022
@mizdra mizdra changed the title Revive rule's documentation links Print rule's documentation links Feb 24, 2022
@mizdra
Copy link
Owner Author

mizdra commented Feb 25, 2022

After some investigation, it turns out that it's not a bug of supports-hyperlinks, but a specification in Node.js.

// - However, due to the specifications of Node.js, the decision does not work well on worker_threads.
// - So here we use a special environment variable to force the printing mode to be switched.
// ref: https://github.com/chalk/supports-color/issues/97, https://github.com/nodejs/node/issues/26946
FORCE_HYPERLINK: terminalLink.isSupported ? '1' : '0',
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 1

Comment on lines +40 to +52
// The `table` package does not print the terminal link correctly. So eslint-interactive avoids
// this by first printing the table in the `table` package without the link,
// then converting it to a link by replacement.
// ref: https://github.com/gajus/table/issues/113
let result = table.table(rows);
ruleStatistics.forEach((ruleStatistic) => {
const { ruleId } = ruleStatistic;
const ruleMetaData = data?.rulesMeta[ruleId];
const ruleCell = ruleMetaData?.docs?.url
? terminalLink(ruleId, ruleMetaData?.docs.url, { fallback: false })
: ruleId;
result = result.replace(` ${ruleId} `, ` ${ruleCell} `);
});
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround 2

@mizdra
Copy link
Owner Author

mizdra commented Mar 22, 2022

LGTM

@mizdra mizdra merged commit bb57c83 into main Mar 22, 2022
@mizdra mizdra deleted the use-table-package branch March 22, 2022 15:13
@mizdra mizdra removed help wanted Extra attention is needed Status: Blocked Progress on the issue is Blocked labels Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revive documentation links
1 participant