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

fix include linter #478

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

fix include linter #478

wants to merge 1 commit into from

Conversation

GavinFrazar
Copy link

@GavinFrazar GavinFrazar commented Jun 21, 2024

I made a docs PR that moved an include file, but I missed one place where the docs still used the old include file path.
That was this PR:

The linter should have caught that mistake, but it didn't.
Here's the lint docs CI run for that PR: https://github.com/gravitational/teleport/actions/runs/9570290956/job/26384715831

For some reason, setting "source" on the vfile messages makes it not report an error consistently.
I have no idea why.
Depending on the surrounding context, it sometimes caught the error or sometimes did not.
For example, this is not caught (the token.mdx file doesn't exist):

(!docs/pages/includes/database-access/token.mdx!)

but if I add some text before/after, it is caught:

blah blah

(!docs/pages/includes/database-access/token.mdx!)

blah blah

With this fix, it catches the bad include consistently:

content/16.x/docs/pages/database-access/enroll-aws-databases/redshift-serverless.mdx
  139:1-139:50  error  Wrong import path docs/pages/includes/database-access/token.mdx in file content/16.x/docs/pages/database-access/enroll-aws-databases/redshift-serverless.mdx.  includes

It also caught other bad includes I was not aware of.

This one should be skipped, I think, since it is marked with {/*lint ignore includes*/}:

content/14.x/docs/pages/contributing/documentation/reference.mdx
    450:1-454:4  error    Wrong import path docs/pages/includes/include.mdx in file content/14.x/docs/pages/contributing/documentation/reference.mdx.     

don't know why it's not skipped.
I can add escapes to fix it, but it appear to be a page that 404 on our docs website contributing link.

This one needs to be fixed on our v14 docs:

content/14.x/docs/pages/database-access/guides/postgres-self-hosted.mdx
  165:88-166:62  error  Includes only works if they are the only content on the line                                                                 includes

I've opened PRs to fix the v14 and v16 issues:

Copy link

vercel bot commented Jun 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2024 11:08pm

@@ -425,7 +425,6 @@ export default function remarkIncludes({
vfile,
startIndex: lastErrorIndex,
ruleId: "includes",
source: "remark-lint",
Copy link
Contributor

Choose a reason for hiding this comment

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

Does naming the source to something beside remark-lint also work? I'm curious about which code reads the source field.

From a quick browse, it looks like the remark-message-control plugin uses it to allow for disabling/enabling linters via comments (this is used by remark-lint). I'm not sure what else uses it.

@iAdramelk, I'm not sure if you're available to answer questions about the update-vfile-messages.ts code—if you are, could you provide some context on what this code does? Thanks!

Copy link
Author

@GavinFrazar GavinFrazar Jun 25, 2024

Choose a reason for hiding this comment

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

Does naming the source to something beside remark-lint also work? I'm curious about which code reads the source field.

I tried changing it to "something" and that didn't fix it for the include link in the redshift guide.

It does affect the example in the 14.x reference that has an ignore comment - if I change source to "something" then it's not ignored and I get:

content/14.x/docs/pages/contributing/documentation/reference.mdx
    450:1-454:4  error    Wrong import path docs/pages/includes/include.mdx in file content/14.x/docs/pages/contributing/documentation/reference.mdx

Otherwise it ignores it correctly.

Having now learned what it's for, I tried this in the redshift serverless file:

## Step 3/4. Install and start the Teleport Database Service

{/*lint enable includes*/} 

(!docs/pages/includes/database-access/token.mdx!)

But it still doesn't detect the error.

Some more info I learned: in general, if a broken include path is followed by another include path that's valid, and remark-lint is in the source, then it doesn't throw an error, e.g.:


(!docs/pages/includes/bad.mdx!)

(!docs/pages/includes/good.mdx!)

Inserting text after bad.mdx above will catch the invalid include path.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ptgott AFAIR source is only used in reporting. See the right side of this screenshot for example: https://github.com/remarkjs/remark-lint/raw/main/screenshot.png

For my knowledge it should not affect actual linter results in any way.

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.

3 participants