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: ignore code blocks #30

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

bencromwell
Copy link
Contributor

@bencromwell bencromwell commented Nov 14, 2024

In some of the Katapult docs we have, for instance:

- `virtual_machine.network_interfaces[].ipv4_addresses[]...`

and some xml examples in a triple-backtick delimited block. At the moment these are currently being checked by Vale.

This is being caused by our set of BlockIgnores.

@bencromwell bencromwell marked this pull request as draft November 14, 2024 08:28
Comment on lines -23 to -27
[*.md]
# These rules exist to allow JSX /HTML in markdown files.
BlockIgnores = (?s) *(<[^>]*>)
BlockIgnores = (?s) *(import.*?\n)
BlockIgnores = (?s) *(export.*?\))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what to do here to achieve the objective of the comment whilst letting it correctly not parse code blocks and such.

Copy link
Contributor

@ikadix ikadix Nov 14, 2024

Choose a reason for hiding this comment

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

I'm not entirely sure what you mean by this comment ? More specifically I don't immediately see how your added rules prevent these from working ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To sum up - with these BlockIgnores in, for whatever reason it prevents Vale from correctly ignoring code blocks like it would by default for markdown or mdx treated as markdown.

But removing them makes it lint JSX, for example:

<ThemedImage
  alt="Web Form for choosing a package."
  sources={{
    light: "/img/compute_how_to_upgrade_downgrade_package_light.png",
    dark: "/img/compute_how_to_upgrade_downgrade_package_dark.png",
  }}
/>

With this tag, it would pick up "png should be PNG", so it's incorrectly linting the tag.

The solution, imperfect as it is, appears to be to have a project with a tag such as that use its own BlockIgnore:

# Custom tags for JSX/MDX. New lines are supported, see https://vale.sh/docs/topics/config/#blockignores 
BlockIgnores = (?s) *<ThemedImage\b[^>]*>(.*?)<\/ThemedImage>|<ThemedImage\b[^>]*\/>

for each tag it wants to do (a little painful).

@bencromwell
Copy link
Contributor Author

bencromwell commented Nov 14, 2024

Test case

case1.md

# Setting up the Go client

Vale should not lint this code block.

```go
package main

import "github.com/krystal/go-katapult/next/core"

func main() {
	katapult, err := core.NewClientWithResponses(
		d.katapultURL,
		d.katapultToken,
	)

	if err != nil {
		log.Fatalf("failed to create katapult client: %v", err)
	}

	// Use the client - `katapult`.
}
```

Vale should report a problem with the lowercase "k" here:

Hello katapult!

@bencromwell
Copy link
Contributor Author

Fix looks like handling each tag individually

BlockIgnores = (?s) *<ThemedImage\b[^>]*>(.*?)<\/ThemedImage>|<ThemedImage\b[^>]*\/>

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