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

Ignore times in common formats (e.g. 2:41PM) #24

Closed
4 tasks done
danielgolden opened this issue Jul 25, 2022 · 5 comments · Fixed by #25
Closed
4 tasks done

Ignore times in common formats (e.g. 2:41PM) #24

danielgolden opened this issue Jul 25, 2022 · 5 comments · Fixed by #25
Labels
💪 phase/solved Post is done 🦋 type/enhancement This is great to have

Comments

@danielgolden
Copy link
Contributor

danielgolden commented Jul 25, 2022

Initial checklist

Affected packages and versions

5.1.0

Link to runnable example

https://peaceful-mclean-b30fef.netlify.app

Steps to reproduce

  1. Open the runnable example.
  2. In the text field, paste the following string: Good morning, today is 2:41pm.
  3. In the right pane, you'll see that a warning is returned for the time of day.

Expected behavior

TL;DR: I expect that the retext-spell plugin wouldn't return any warnings for times in common formats like 2:41pm.

Description

First, retext and unified as a whole is an incredible project. Thank you for making and maintaining this.

I'm working on creating a JavaScript prose linter for anyone who writes copy in our UI. It's accessible via a browser or in a Figma Plugin since it's primarily meant for product designers to use. The prose linter that I've created uses many existing retext plugins (like retext-spell) and several custom plugins for rules to help our teams adhere to our writing style guide. Since my company is a software monitoring tool, we often use times (e.g. 2:41pm) in our UI and it's very plausible that folks will attempt to use my plugin on text layers that contain times.

I'm taking advantage of the ignoreDigits option and it's working wonderfully! I was hoping to find another option that allows consumers of the project to control whether to ignore words that contain any digits as opposed to only digits (as the ignoreDigits option does).

Affected runtime and version

node@14.15.5

Affected package manager and version

npm@8.10.0

Affected OS and version

macOS Monterey 12.2.1

Build and bundle tools

webpack, Vite

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jul 25, 2022
@wooorm
Copy link
Member

wooorm commented Jul 25, 2022

Good idea.
If you are interested in a PR, it could be solved here:

return ignore.includes(word) || (ignoreDigits && /^\d+$/.test(word))
.

@wooorm wooorm added the 👍 phase/yes Post is accepted and can be worked on label Jul 25, 2022
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Jul 25, 2022
@github-actions

This comment has been minimized.

@danielgolden
Copy link
Contributor Author

Awesome. Thanks for the feedback! I'm definitely interested.
I'll try to submit a PR for it this week.

wooorm pushed a commit that referenced this issue Jul 30, 2022
Co-authored-by: Titus Wormer <tituswormer@gmail.com>

Closes GH-24.
Closes GH-25.

Reviewed-by: Titus Wormer <tituswormer@gmail.com>
@github-actions

This comment has been minimized.

@wooorm wooorm added 🦋 type/enhancement This is great to have 💪 phase/solved Post is done labels Jul 30, 2022
@github-actions github-actions bot removed the 👍 phase/yes Post is accepted and can be worked on label Jul 30, 2022
@wooorm
Copy link
Member

wooorm commented Jul 30, 2022

Released in 5.3.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done 🦋 type/enhancement This is great to have
Development

Successfully merging a pull request may close this issue.

2 participants