From dd1def02e487a2a43903aa88022b266a3ecbef4d Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Thu, 28 Mar 2024 17:00:56 -0400 Subject: [PATCH] Move parse_string_pattern in cli to StringPattern::parse in lib 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. --- cli/src/cli_util.rs | 10 +------- cli/src/commands/branch.rs | 10 ++++---- cli/src/commands/git.rs | 9 ++++--- cli/src/commands/tag.rs | 4 ++-- lib/src/str_util.rs | 48 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 22 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index c11b86fd2..8ba78e29f 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -58,7 +58,7 @@ use jj_lib::revset::{ use jj_lib::rewrite::restore_tree; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; use jj_lib::signing::SignInitError; -use jj_lib::str_util::{StringPattern, StringPatternParseError}; +use jj_lib::str_util::StringPattern; use jj_lib::transaction::Transaction; use jj_lib::view::View; use jj_lib::working_copy::{ @@ -1651,14 +1651,6 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> { Ok(()) } -pub fn parse_string_pattern(src: &str) -> Result { - if let Some((kind, pat)) = src.split_once(':') { - StringPattern::from_str_kind(pat, kind) - } else { - Ok(StringPattern::exact(src)) - } -} - /// Resolves revsets into revisions for use; useful for rebases or operations /// that take multiple parents. pub fn resolve_all_revs( diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 4520e16fe..7417abd5a 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -27,9 +27,7 @@ use jj_lib::revset::{self, RevsetExpression}; use jj_lib::str_util::StringPattern; use jj_lib::view::View; -use crate::cli_util::{ - parse_string_pattern, CommandHelper, RemoteBranchName, RemoteBranchNamePattern, RevisionArg, -}; +use crate::cli_util::{CommandHelper, RemoteBranchName, RemoteBranchNamePattern, RevisionArg}; use crate::command_error::{user_error, user_error_with_hint, CommandError}; use crate::formatter::Formatter; use crate::ui::Ui; @@ -78,7 +76,7 @@ pub struct BranchDeleteArgs { /// By default, the specified name matches exactly. Use `glob:` prefix to /// select branches by wildcard pattern. For details, see /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. - #[arg(required_unless_present_any(&["glob"]), value_parser = parse_string_pattern)] + #[arg(required_unless_present_any(&["glob"]), value_parser = StringPattern::parse)] pub names: Vec, /// Deprecated. Please prefix the pattern with `glob:` instead. @@ -117,7 +115,7 @@ pub struct BranchListArgs { /// By default, the specified name matches exactly. Use `glob:` prefix to /// select branches by wildcard pattern. For details, see /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. - #[arg(value_parser = parse_string_pattern)] + #[arg(value_parser = StringPattern::parse)] pub names: Vec, /// Show branches whose local targets are in the given revisions. @@ -140,7 +138,7 @@ pub struct BranchForgetArgs { /// By default, the specified name matches exactly. Use `glob:` prefix to /// select branches by wildcard pattern. For details, see /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. - #[arg(required_unless_present_any(&["glob"]), value_parser = parse_string_pattern)] + #[arg(required_unless_present_any(&["glob"]), value_parser = StringPattern::parse)] pub names: Vec, /// Deprecated. Please prefix the pattern with `glob:` instead. diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index c186dc7f0..82c035bfc 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -41,9 +41,8 @@ use jj_lib::workspace::Workspace; use maplit::hashset; use crate::cli_util::{ - parse_string_pattern, print_trackable_remote_branches, short_change_hash, short_commit_hash, - start_repo_transaction, CommandHelper, RevisionArg, WorkspaceCommandHelper, - WorkspaceCommandTransaction, + print_trackable_remote_branches, short_change_hash, short_commit_hash, start_repo_transaction, + CommandHelper, RevisionArg, WorkspaceCommandHelper, WorkspaceCommandTransaction, }; use crate::command_error::{ user_error, user_error_with_hint, user_error_with_message, CommandError, @@ -158,7 +157,7 @@ pub struct GitFetchArgs { /// /// By default, the specified name matches exactly. Use `glob:` prefix to /// expand `*` as a glob. The other wildcard characters aren't supported. - #[arg(long, short, default_value = "glob:*", value_parser = parse_string_pattern)] + #[arg(long, short, default_value = "glob:*", value_parser = StringPattern::parse)] branch: Vec, /// The remote to fetch from (only named remotes are supported, can be /// repeated) @@ -203,7 +202,7 @@ pub struct GitPushArgs { /// By default, the specified name matches exactly. Use `glob:` prefix to /// select branches by wildcard pattern. For details, see /// https://martinvonz.github.io/jj/latest/revsets#string-patterns. - #[arg(long, short, value_parser = parse_string_pattern)] + #[arg(long, short, value_parser = StringPattern::parse)] branch: Vec, /// Push all branches (including deleted branches) #[arg(long)] diff --git a/cli/src/commands/tag.rs b/cli/src/commands/tag.rs index 6103f0bd8..fb1d46a80 100644 --- a/cli/src/commands/tag.rs +++ b/cli/src/commands/tag.rs @@ -14,7 +14,7 @@ use jj_lib::str_util::StringPattern; -use crate::cli_util::{parse_string_pattern, CommandHelper}; +use crate::cli_util::CommandHelper; use crate::command_error::CommandError; use crate::ui::Ui; @@ -33,7 +33,7 @@ pub struct TagListArgs { /// By default, the specified name matches exactly. Use `glob:` prefix to /// select tags by wildcard pattern. For details, see /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. - #[arg(value_parser = parse_string_pattern)] + #[arg(value_parser = StringPattern::parse)] pub names: Vec, } diff --git a/lib/src/str_util.rs b/lib/src/str_util.rs index ffe58e0a4..1df3fdf2f 100644 --- a/lib/src/str_util.rs +++ b/lib/src/str_util.rs @@ -50,6 +50,19 @@ impl StringPattern { StringPattern::Substring(String::new()) } + /// Parses the given string as a `StringPattern`. Everything before the + /// first ":" is considered the string's prefix. If the prefix is "exact:", + /// "glob:", or "substring:", a pattern of the specified kind is returned. + /// Returns an error if the string has an unrecognized prefix. Otherwise, a + /// `StringPattern::Exact` is returned. + pub fn parse(src: &str) -> Result { + if let Some((kind, pat)) = src.split_once(':') { + StringPattern::from_str_kind(pat, kind) + } else { + Ok(StringPattern::exact(src)) + } + } + /// Creates pattern that matches exactly. pub fn exact(src: impl Into) -> Self { StringPattern::Exact(src.into()) @@ -163,4 +176,39 @@ mod tests { Some("*[*]*".into()) ); } + + #[test] + fn test_parse() { + // Parse specific pattern kinds. + assert_eq!( + StringPattern::parse("exact:foo").unwrap(), + StringPattern::from_str_kind("foo", "exact").unwrap() + ); + assert_eq!( + StringPattern::parse("glob:foo*").unwrap(), + StringPattern::from_str_kind("foo*", "glob").unwrap() + ); + assert_eq!( + StringPattern::parse("substring:foo").unwrap(), + StringPattern::from_str_kind("foo", "substring").unwrap() + ); + + // Parse a pattern that contains a : itself. + assert_eq!( + StringPattern::parse("exact:foo:bar").unwrap(), + StringPattern::from_str_kind("foo:bar", "exact").unwrap() + ); + + // If no kind is specified, the input is treated as an exact pattern. + assert_eq!( + StringPattern::parse("foo").unwrap(), + StringPattern::from_str_kind("foo", "exact").unwrap() + ); + + // Parsing an unknown prefix results in an error. + assert!(matches! { + StringPattern::parse("unknown-prefix:foo"), + Err(StringPatternParseError::InvalidKind(_)) + }); + } }