completion: teach log about files

cli/src/commands/log.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use clap_complete::ArgValueCandidates;
+use clap_complete::ArgValueCompleter;
 use jj_lib::backend::CommitId;
 use jj_lib::config::ConfigGetError;
 use jj_lib::config::ConfigGetResultExt as _;
@@ -65,7 +66,10 @@ pub(crate) struct LogArgs {
     #[arg(long, short, add = ArgValueCandidates::new(complete::all_revisions))]
     revisions: Vec<RevisionArg>,
     /// Show revisions modifying the given paths
-    #[arg(value_hint = clap::ValueHint::AnyPath)]
+    #[arg(
+        value_hint = clap::ValueHint::AnyPath,
+        add = ArgValueCompleter::new(complete::log_files),
+    )]
     paths: Vec<String>,
     /// Show revisions in the opposite order (older revisions first)
     #[arg(long)]

cli/src/complete.rs

@@ -638,6 +638,17 @@ pub fn interdiff_files(current: &std::ffi::OsStr) -> Vec<CompletionCandidate> {
     })
 }
 
+/// Specific function for completing file paths for `jj log`
+pub fn log_files(current: &std::ffi::OsStr) -> Vec<CompletionCandidate> {
+    let mut rev = parse::log_revisions().join(")|(");
+    if rev.is_empty() {
+        rev = "@".into();
+    } else {
+        rev = format!("latest(heads(({rev})))"); // limit to one
+    };
+    all_files_from_rev(rev, current)
+}
+
 /// Shell out to jj during dynamic completion generation
 ///
 /// In case of errors, print them and early return an empty vector.
@@ -773,36 +784,41 @@ impl JjBuilder {
 /// multiple times, the parsing will pick any of the available ones, while the
 /// actual execution of the command would fail.
 mod parse {
-    fn parse_flag(candidates: &[&str], mut args: impl Iterator<Item = String>) -> Option<String> {
+    pub(super) fn parse_flag<'a>(
-        for arg in args.by_ref() {
+        candidates: &'a [&str],
-            // -r REV syntax
+        mut args: impl Iterator<Item = String> + 'a,
-            if candidates.contains(&arg.as_ref()) {
+    ) -> impl Iterator<Item = String> + 'a {
-                match args.next() {
+        std::iter::from_fn(move || {
-                    Some(val) if !val.starts_with('-') => return Some(val),
+            for arg in args.by_ref() {
-                    _ => return None,
+                // -r REV syntax
+                if candidates.contains(&arg.as_ref()) {
+                    match args.next() {
+                        Some(val) if !val.starts_with('-') => return Some(val),
+                        _ => return None,
+                    }
                 }
+
+                // -r=REV syntax
+                if let Some(value) = candidates.iter().find_map(|candidate| {
+                    let rest = arg.strip_prefix(candidate)?;
+                    match rest.strip_prefix('=') {
+                        Some(value) => Some(value),
+
+                        // -rREV syntax
+                        None if candidate.len() == 2 => Some(rest),
+
+                        None => None,
+                    }
+                }) {
+                    return Some(value.into());
+                };
             }
-
+            None
-            // -r=REV syntax
+        })
-            if let Some(value) = candidates.iter().find_map(|candidate| {
-                let rest = arg.strip_prefix(candidate)?;
-                match rest.strip_prefix('=') {
-                    Some(value) => Some(value),
-
-                    // -rREV syntax
-                    None if candidate.len() == 2 => Some(rest),
-
-                    None => None,
-                }
-            }) {
-                return Some(value.into());
-            };
-        }
-        None
     }
 
     pub fn parse_revision_impl(args: impl Iterator<Item = String>) -> Option<String> {
-        parse_flag(&["-r", "--revision"], args)
+        parse_flag(&["-r", "--revision"], args).next()
     }
 
     pub fn revision() -> Option<String> {
@@ -817,8 +833,10 @@ mod parse {
     where
         T: Iterator<Item = String>,
     {
-        let from = parse_flag(&["-f", "--from"], args())?;
+        let from = parse_flag(&["-f", "--from"], args()).next()?;
-        let to = parse_flag(&["-t", "--to"], args()).unwrap_or_else(|| "@".into());
+        let to = parse_flag(&["-t", "--to"], args())
+            .next()
+            .unwrap_or_else(|| "@".into());
 
         Some((from, to))
     }
@@ -832,10 +850,17 @@ mod parse {
     // the files changed only in some other revision in the range between
     // --from and --to cannot be squashed into --to like that.
     pub fn squash_revision() -> Option<String> {
-        if let Some(rev) = parse_flag(&["-r", "--revision"], std::env::args()) {
+        if let Some(rev) = parse_flag(&["-r", "--revision"], std::env::args()).next() {
             return Some(rev);
         }
-        parse_flag(&["-f", "--from"], std::env::args())
+        parse_flag(&["-f", "--from"], std::env::args()).next()
+    }
+
+    // Special parse function only for `jj log`. It has a --revisions flag,
+    // instead of the usual --revision, and it can be supplied multiple times.
+    pub fn log_revisions() -> Vec<String> {
+        let candidates = &["-r", "--revisions"];
+        parse_flag(candidates, std::env::args()).collect()
     }
 }
 
@@ -915,4 +940,25 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn test_parse_multiple_flags() {
+        let candidates = &["-r", "--revisions"];
+        let args = &[
+            "unrelated_arg_at_the_beginning",
+            "-r",
+            "1",
+            "--revisions",
+            "2",
+            "-r=3",
+            "--revisions=4",
+            "unrelated_arg_in_the_middle",
+            "-r5",
+            "unrelated_arg_at_the_end",
+        ];
+        let flags: Vec<_> =
+            parse::parse_flag(candidates, args.iter().map(|a| a.to_string())).collect();
+        let expected = ["1", "2", "3", "4", "5"];
+        assert_eq!(flags, expected);
+    }
 }

cli/tests/test_completion.rs

@@ -788,4 +788,35 @@ fn test_files() {
     f_dir/
     f_modified
     ");
+
+    let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "log", "f_"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r"
+    f_added
+    f_added_2
+    f_dir/
+    f_modified
+    f_not_yet_renamed
+    f_renamed
+    f_unchanged
+    ");
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &[
+            "--",
+            "jj",
+            "log",
+            "-r=first",
+            "--revisions",
+            "conflicted",
+            "f_",
+        ],
+    );
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r"
+    f_added_2
+    f_deleted
+    f_dir/
+    f_modified
+    f_not_yet_renamed
+    f_unchanged
+    ");
 }