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

Add support for @upper_case @lower_case captures #838

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ctdunc
Copy link

@ctdunc ctdunc commented Jan 17, 2025

Add support for @upper_case @lower_case captures.

Issue: #836

Description

This adds really basic support for the feature discussed in the linked issue.
I tested it with my SQL capture, and it works, but I don't know if adding SQL as a language is in scope for this PR and there are a lot of other issues with my query (it's still kinda ugly).

Checklist

Checklist before merging:

  • CHANGELOG.md updated
  • README.md up-to-date

lower_case_level += 1;
*atom = Atom::Empty;
if upper_case_level > 0 {
panic!("Cannot use @upper_case inside of a node captured by @lower_case!");
Copy link
Author

Choose a reason for hiding this comment

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

This is probably not ideal, but I wasn't sure what the preferred way of handling such errors is. There may also be times where users want to capitalize all but a subset of a node, which this would need to be changed to support.

@ctdunc ctdunc force-pushed the feature/capitalization branch from bbbf44b to 5da7294 Compare January 17, 2025 21:15
how_capitalize: HowCapitalize::UpperCase,
}
} else if lower_case_level > 0 {
*atom = Atom::Leaf {
Copy link
Author

Choose a reason for hiding this comment

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

these derefs feel wrong, but I'm not really a Rust expert, so I am not sure whether these are copying. I will learn from whatever feedback you can provide on this

} => {
// TODO don't be stupid with derefs
*atom = Atom::Leaf {
content: (*content).to_string(),
Copy link
Author

Choose a reason for hiding this comment

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

the derefs here feel bad. I'm sure there's a rustier way to do this.

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.

1 participant