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

Move parse_string_pattern in cli to StringPattern::parse in lib #3393

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

emesterhazy
Copy link
Collaborator

This commit moves the parse_string_pattern helper function into the str_util module in jj lib and adds tests for it.

I'd like to reuse this code in a function defined by UserSettings, which is part of the jj lib crate and cannot use functions from the cli crate.

This commit moves the parse_string_pattern helper function into the
str_util module in jj lib and adds tests for it.

I'd like to reuse this code in a function defined by `UserSettings`, which is
part of the jj lib crate and cannot use functions from the cli crate.
@emesterhazy
Copy link
Collaborator Author

Hi Yuya, would you please take a look at this PR? I'd like to do this refactor so that it's easier to parse a string pattern when I implement your suggestion on my other PR for the advance-branches feature.

@emesterhazy emesterhazy requested a review from yuja March 28, 2024 21:42
Copy link
Collaborator

@thoughtpolice thoughtpolice left a comment

Choose a reason for hiding this comment

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

LGTM; let's let Yuya chime in too

Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

lgtm, thanks.

reuse this code in a function defined by UserSettings

Just fyi, not all config keys should have to be defined in the UserSettings. If there are no callers in lib, I personally wouldn't add the one to UserSettings.

lib/src/str_util.rs Show resolved Hide resolved
@emesterhazy emesterhazy merged commit dd1def0 into main Mar 29, 2024
16 checks passed
@emesterhazy emesterhazy deleted the push-myzuxsmzzpnx branch March 29, 2024 12:48
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.

3 participants