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

Use AST parser to validate alt text - markdown-lint #33

Merged
merged 13 commits into from
Oct 13, 2023

Conversation

kendallgassner
Copy link
Collaborator

@kendallgassner kendallgassner commented Oct 4, 2023

What

Closes #30

I updated the alt-text-bot to use an AST (markdown-lint) to parse images for bad alt text. Using an AST allows us to ignore any images defined in code blocks.

Why is this cool

  • In the future, we can add additional linting rules
  • The workflow is easier to read in javascript
  • We can test more easily
  • We can ensure that there aren't false positives
  • We can provide more feedback. For example, it is easier to indicate what line the alt text is found on.

src/index.test.js Outdated Show resolved Hide resolved
src/index.test.js Outdated Show resolved Hide resolved
src/index.test.js Outdated Show resolved Hide resolved
@kendallgassner kendallgassner marked this pull request as ready for review October 11, 2023 15:08
@kendallgassner kendallgassner requested a review from a team as a code owner October 11, 2023 15:08
action.yml Outdated
runs:
using: "composite"
steps:
- name: Runs alt text check and adds comment
run: |
source ${{ github.action_path }}/flag-alt-text.sh
source ${{ github.action_path }}/queries.sh
source ${{ github.action_path }}/flag-alt-text.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we can delete this given the file has been removed!

Copy link
Contributor

Choose a reason for hiding this comment

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

I am testing this branch in one of my repos! (Related workflow error)

@@ -0,0 +1,127 @@
import { validate } from "./validate.js";
Copy link
Contributor

Choose a reason for hiding this comment

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

Beautiful test coverage!

elif [[ $type = discussion_description ]]; then
addDiscussionComment $discussion_node_id "$message"
elif [[ $type = discussion_comment ]]; then
addDiscussionComment $discussion_node_id "$message" $reply_to_id
Copy link
Contributor

Choose a reason for hiding this comment

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

I would love if we can eventually move all of this bash logic into a well-tested node script! Out of scope here though :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed!!

src/index.js Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@kendallgassner kendallgassner requested a review from khiga8 October 11, 2023 23:29
action.yml Outdated Show resolved Hide resolved
@kendallgassner
Copy link
Collaborator Author

kendallgassner commented Oct 11, 2023

Proof of concept run: khiga8/link-checker#9

@khiga8 ... i had to check in node_modules..... but it works

@khiga8
Copy link
Contributor

khiga8 commented Oct 12, 2023

Eeep sorry I didn't get to review this today, I'll take a look first thing tomorrow!

Copy link
Contributor

@khiga8 khiga8 left a comment

Choose a reason for hiding this comment

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

Great work!

@kendallgassner
Copy link
Collaborator Author

Checking in node_modules as advised on github action documentation.

@kendallgassner kendallgassner merged commit 602a5ef into main Oct 13, 2023
1 check passed
@kendallgassner kendallgassner deleted the testing-markdown-lint branch October 13, 2023 16:41
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.

Ignore images in code snippets
2 participants