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

[BUG] Fails to parse commits with whitespace in scope #4

Open
orhun opened this issue Jan 20, 2025 · 7 comments
Open

[BUG] Fails to parse commits with whitespace in scope #4

orhun opened this issue Jan 20, 2025 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@orhun
Copy link

orhun commented Jan 20, 2025

Describe the bug
The parser fails to parse commits such as:

"feat(some scope): message"

To Reproduce

See #5

Expected behavior

Successful parse.

Additional context
Originally reported at release-plz/release-plz#1996

@oknozor
Copy link
Collaborator

oknozor commented Jan 21, 2025

This is intentional, as far as I understand the spec, space are not allowed in scope.

"A scope MAY be provided after a type. A scope MUST consist of a noun describing a section of the codebase surrounded by parenthesis, e.g., fix(parser):"

I might be wrong though but my understanding is that a scope must be a single noun.

@orhun
Copy link
Author

orhun commented Jan 21, 2025

Well yeah, it reads like that...

As a reference I guess git-conventional already supports that.

A user needs this feature in orhun/git-cliff#1008 so how do you want to proceed here? 👀 Would it be possible to implement it or do you want to adhere to the spec?

@oknozor
Copy link
Collaborator

oknozor commented Jan 22, 2025

Hey again @orhun,

I definitely prefer the crate to be strict with the spec, I would accept a PR if this is behind a feature flag though.

@orhun
Copy link
Author

orhun commented Jan 22, 2025

I would be interested in contributing this feature - what would be the feature flag called?

P.S. inviting @marcoieni (maintainer of release-plz) to the discussion.

@oknozor
Copy link
Collaborator

oknozor commented Jan 23, 2025

lax-scope I guess

@oknozor
Copy link
Collaborator

oknozor commented Jan 23, 2025

Another alternative would be to add a function allowing post parsing validation via some predicates, this would not require a feature flag though.

Also this would allow to support many user specific validations such as cocogitto/cocogitto#434

@orhun
Copy link
Author

orhun commented Jan 29, 2025

Sure, that sounds like a more flexible solution. I'm not sure how that would look like in the API side though.

Do you mean adding a e.g. post_process function to conventional_commit_parser::parse for manipulating the parsed commit?

In the meantime I looked into supporting this with a feature flag and it seems like it can be configured on the pest-level:

 commit_type = { ASCII_ALPHA+ }
 scope = {  (parent_left ~ scope_content ~ parent_right) }
 breaking_change_mark = { "!"? }
-scope_content = ${ ( no_whitespace ~ no_parenthesis ~ !NEWLINE ~ ANY)+ }
+scope_content = ${ ( no_parenthesis ~ !NEWLINE ~ ANY)+ }

Is there any other way to make it configurable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants