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

feat: variadic input helpers #284

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

didavid61202
Copy link
Collaborator

@didavid61202 didavid61202 commented Apr 1, 2023

Update & Proposal

Update input helpers to take variadic arguments, eg: exactly('foo', maybe('bar'), 'baz'...), enabling a more concise syntax and improving the overall readability of the code.

Currently, the input helpers take only one argument of either type string or another Input. To concatenate different RegExp patterns, users need to use the .and() chaining helper repeatedly. This approach is very descriptive but can also make the code verbose and difficult to read when dealing with multiple input helpers in more complex scenario.

To improve the syntax and readability, I want to propose an update to all input helpers to variadic function that take one or more string or Input as arguments.
And this update will maintain backward compatibility, as the new helpers still accept a single argument of type string or Input, and still can chain using .and(). However, users will also have the option to pass multiple arguments to the functions, which will be concatenated accordingly.

please let me know your suggestion, feedback, and input.

Updated inputs

  • createRegExp (last array arg as flags)
  • maybe
  • exactly
  • oneOrMore
  • .and
  • .or
  • .after
  • .before
  • .notAfter
  • .notBefore

Example

original semver example

const regExp = createRegExp(
  oneOrMore(digit)
    .groupedAs('major')
    .and('.')
    .and(oneOrMore(digit).groupedAs('minor'))
    .and(exactly('.').and(oneOrMore(char).groupedAs('patch')).optionally())
)

with this update, can be simplified to:

  const regExp2 = createRegExp(
      oneOrMore(digit).groupedAs('major'),
      '.',
      oneOrMore(digit).groupedAs('minor'),
      maybe('.', oneOrMore(char).groupedAs('patch'))
    )

Related issues

#237

  • add/update tests
  • update docs
  • update jsdoc comments

@vercel
Copy link

vercel bot commented Apr 1, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @danielroe on Vercel.

@danielroe first needs to authorize it.

@what-the-diff
Copy link

what-the-diff bot commented Apr 1, 2023

PR Summary

  • Updated anyOf type
    Changed the type of anyOf to accept an array instead of a spread for easier usage
  • Added types for inputs
    Added types for all inputs, including optional and repeatable ones, increasing code readability
  • Bug fixes in code
    Fixed various bugs in the code, such as missing return statements, enhancing stability
  • Enhanced createRegExp
    Added variadic args and flags to createRegExp for more flexible usage
  • Multichar input bug fix
    Fixed a bug related to improper extraction of multichar inputs when chained with other inputs, improving overall functionality

@what-the-diff
Copy link

what-the-diff bot commented Apr 1, 2023

PR Summary

  • Changed anyOf type to accept array
    The function now accepts an array instead of a spread for better handling.
  • Added types for all inputs
    This includes optional and repeatable inputs for better code stability.
  • Fixed bugs in the code
    Resolved issues such as missing return statements for improved performance.
  • Added variadic args and flags to createRegExp
    This update allows more flexible handling of arguments for creating regular expressions.
  • Fixed multichar input bug in chained methods
    Resolved issues with multicharacter inputs when using and and or chained methods.
  • Updated tests for new features and fixes
    Tests have been updated to cover the new features and fixes for productive and reliable results.

@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (5172d2b) 100.00% compared to head (a50f662) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #284   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          467       520   +53     
  Branches        82        87    +5     
=========================================
+ Hits           467       520   +53     
Impacted Files Coverage Δ
src/core/inputs.ts 100.00% <100.00%> (ø)
src/core/internal.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vercel
Copy link

vercel bot commented Apr 1, 2023

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

Name Status Preview Comments Updated (UTC)
magic-regexp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 10, 2023 10:47am

@danielroe
Copy link
Member

I really like this ❤️ It makes writing regexp much less verbose - great DX.

@didavid61202
Copy link
Collaborator Author

I really like this ❤️ It makes writing regexp much less verbose - great DX.

@danielroe Thanks!
Always strive for better DX 🚀
Just complete updating the docs and JSDoc comment to all related input helpers too, please check on them if you got some spare time, then I think it's good to go 👍

@xRSquared
Copy link
Contributor

Using variadic functions is an elegant solution. I love it!

didavid61202 and others added 2 commits April 11, 2023 00:15
Co-authored-by: Daniel Roe <daniel@roe.dev>
Co-authored-by: Daniel Roe <daniel@roe.dev>
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

❤️

@danielroe danielroe merged commit 00dab95 into unjs:main Apr 13, 2023
6 checks passed
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.

None yet

4 participants