Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions josh-core/src/filter/text.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use crate::JoshResult;
use regex::Regex;
use std::cell::RefCell;
use std::collections::HashMap;

pub fn transform_with_template(
re: &Regex,
template: &str,
input: &str,
globals: &HashMap<String, String>,
) -> JoshResult<String> {
let first_error: RefCell<Option<crate::JoshError>> = RefCell::new(None);

let result = re
.replace_all(input, |caps: &regex::Captures| {
// Build a HashMap with all named captures and globals
// We need to store the string values to keep them alive for the HashMap references
let mut string_storage: HashMap<String, String> = HashMap::new();

// Collect all named capture values
for name in re.capture_names().flatten() {
if let Some(m) = caps.name(name) {
string_storage.insert(name.to_string(), m.as_str().to_string());
}
}

// Build the HashMap for strfmt with references to the stored strings
let mut vars: HashMap<String, &dyn strfmt::DisplayStr> = HashMap::new();

// Add all globals first (lower priority)
for (key, value) in globals {
vars.insert(key.clone(), value as &dyn strfmt::DisplayStr);
}

// Add all named captures (higher priority - will overwrite globals if there's a conflict)
for (key, value) in &string_storage {
vars.insert(key.clone(), value as &dyn strfmt::DisplayStr);
}

// Format the template, propagating errors
match strfmt::strfmt(template, &vars) {
Ok(s) => s,
Err(e) => {
let mut error = first_error.borrow_mut();
if error.is_none() {
*error = Some(e.into());
}
caps[0].to_string()
}
}
})
.into_owned();

match first_error.into_inner() {
Some(e) => Err(e),
None => Ok(result),
}
Comment on lines +16 to +57
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The function contains complex template formatting logic (lines 18-50) that is not used in the current implementation. The only caller passes an empty string as the template, making the strfmt logic, string_storage, vars HashMap, and error handling for template formatting dead code. Consider simplifying to a basic regex replace function or documenting the intended future use of template parameters.

Suggested change
// Build a HashMap with all named captures and globals
// We need to store the string values to keep them alive for the HashMap references
let mut string_storage: HashMap<String, String> = HashMap::new();
// Collect all named capture values
for name in re.capture_names().flatten() {
if let Some(m) = caps.name(name) {
string_storage.insert(name.to_string(), m.as_str().to_string());
}
}
// Build the HashMap for strfmt with references to the stored strings
let mut vars: HashMap<String, &dyn strfmt::DisplayStr> = HashMap::new();
// Add all globals first (lower priority)
for (key, value) in globals {
vars.insert(key.clone(), value as &dyn strfmt::DisplayStr);
}
// Add all named captures (higher priority - will overwrite globals if there's a conflict)
for (key, value) in &string_storage {
vars.insert(key.clone(), value as &dyn strfmt::DisplayStr);
}
// Format the template, propagating errors
match strfmt::strfmt(template, &vars) {
Ok(s) => s,
Err(e) => {
let mut error = first_error.borrow_mut();
if error.is_none() {
*error = Some(e.into());
}
caps[0].to_string()
}
}
})
.into_owned();
match first_error.into_inner() {
Some(e) => Err(e),
None => Ok(result),
}
// Since template formatting is not used, just return the matched string
caps[0].to_string()
})
.into_owned();
Ok(result)

Copilot uses AI. Check for mistakes.
}
14 changes: 13 additions & 1 deletion josh-core/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,11 +577,23 @@ pub fn unapply_filter(
}
};

let mut apply = filter::Apply::from_tree(new_tree.clone());

if change_ids.is_some() {
let new_message = filter::text::transform_with_template(
&regex::Regex::new(&"(?m)^Change: [^ ]+")?,
&"",
module_commit.message_raw().unwrap(),
&std::collections::HashMap::new(),
)?;
Comment on lines +583 to +588
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The regex pattern &\"(?m)^Change: [^ ]+\" has an unnecessary reference operator before the string literal. It should be \"(?m)^Change: [^ ]+\" (without the leading &). The Regex::new() function expects &str, not &&str.

Copilot uses AI. Check for mistakes.
Comment on lines +583 to +588
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The regex pattern (?m)^Change: [^ ]+ will match and remove the line but leave the newline character, resulting in blank lines in commit messages. Consider using (?m)^Change: [^ ]+\\n? to also remove the trailing newline, or (?m)^Change: [^ ]+$\\n? to be more precise about matching end-of-line.

Copilot uses AI. Check for mistakes.
apply = apply.with_message(new_message);
}

ret = rewrite_commit(
transaction.repo(),
&module_commit,
&original_parents,
filter::Apply::from_tree(new_tree.clone()),
apply,
false,
)?;

Expand Down
44 changes: 24 additions & 20 deletions tests/cli/push_stacked_split.t
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ Make multiple changes with Change-Ids for split testing
$ git commit -q -m "Change-Id: 1234"
$ echo "contents2" > file7
$ git add file7
$ git commit -q -m "Change-Id: foo7"
$ git commit -q -m "Change: foo7"
$ echo "contents3" > file2
$ git add file2
$ git commit -q -m "Change-Id: 1235"
$ git log --decorate --graph --pretty="%s %d"
* Change-Id: 1235 (HEAD -> master)
* Change-Id: foo7
* Change: foo7
* Change-Id: 1234
* add file1 (origin/master, origin/HEAD)

Expand All @@ -72,32 +72,32 @@ Push with split mode (should create multiple refs for each change)

Pushed c61c37f4a3d5eb447f41dde15620eee1a181d60b to origin/refs/heads/@changes/master/josh@example.com/1234
To $TESTTMP/remote
* [new branch] c1b55ea7e5f27f82d3565c1f5d64113adf635c2c -> @changes/master/josh@example.com/foo7
* [new branch] 9da166dcfa8650e04c7e39c54a61b7fa0b69ef4f -> @changes/master/josh@example.com/foo7

Pushed c1b55ea7e5f27f82d3565c1f5d64113adf635c2c to origin/refs/heads/@changes/master/josh@example.com/foo7
Pushed 9da166dcfa8650e04c7e39c54a61b7fa0b69ef4f to origin/refs/heads/@changes/master/josh@example.com/foo7
To $TESTTMP/remote
* [new branch] ef7c3c85ad4c5875f308003d42a6e11d9b14aeb9 -> @changes/master/josh@example.com/1235

Pushed ef7c3c85ad4c5875f308003d42a6e11d9b14aeb9 to origin/refs/heads/@changes/master/josh@example.com/1235
To $TESTTMP/remote
* [new branch] 02796cbb12e05f3be9f16c82e4d26542af7e700c -> @heads/master/josh@example.com
* [new branch] 52310103e5d9a8e55df1a7766789a1af04d8601b -> @heads/master/josh@example.com

Pushed 02796cbb12e05f3be9f16c82e4d26542af7e700c to origin/refs/heads/@heads/master/josh@example.com
Pushed 52310103e5d9a8e55df1a7766789a1af04d8601b to origin/refs/heads/@heads/master/josh@example.com

Verify the refs were created in the remote

$ cd ${TESTTMP}/remote
$ git ls-remote . | grep "@" | sort
02796cbb12e05f3be9f16c82e4d26542af7e700c\trefs/heads/@heads/master/josh@example.com (esc)
c1b55ea7e5f27f82d3565c1f5d64113adf635c2c\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
52310103e5d9a8e55df1a7766789a1af04d8601b\trefs/heads/@heads/master/josh@example.com (esc)
9da166dcfa8650e04c7e39c54a61b7fa0b69ef4f\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
c61c37f4a3d5eb447f41dde15620eee1a181d60b\trefs/heads/@changes/master/josh@example.com/1234 (esc)
ef7c3c85ad4c5875f308003d42a6e11d9b14aeb9\trefs/heads/@changes/master/josh@example.com/1235 (esc)

$ git log --all --decorate --graph --pretty="%s %d %H"
* Change-Id: 1235 (@changes/master/josh@example.com/1235) ef7c3c85ad4c5875f308003d42a6e11d9b14aeb9
| * Change-Id: foo7 (@changes/master/josh@example.com/foo7) c1b55ea7e5f27f82d3565c1f5d64113adf635c2c
| | * Change-Id: 1235 (@heads/master/josh@example.com) 02796cbb12e05f3be9f16c82e4d26542af7e700c
| | * Change-Id: foo7 2cbfa8cb8d9a9f1de029fcba547a6e56c742733f
| * (@changes/master/josh@example.com/foo7) 9da166dcfa8650e04c7e39c54a61b7fa0b69ef4f
| | * Change-Id: 1235 (@heads/master/josh@example.com) 52310103e5d9a8e55df1a7766789a1af04d8601b
| | * 48f307ad20210547fdf339d0b0d7ee02bc702c3d
| |/
|/|
* | Change-Id: 1234 (@changes/master/josh@example.com/1234) c61c37f4a3d5eb447f41dde15620eee1a181d60b
Expand All @@ -123,21 +123,25 @@ Test that we can fetch the split refs back
Fetched from remote: origin

$ git log --all --decorate --graph --pretty="%s %d %H"
* Change-Id: 1235 (HEAD -> master, origin/@heads/master/josh@example.com) 3faa5b51d4600be54a2b32e84697e7b32a781a03
* Change-Id: foo7 da80e49d24d110866ce2ec7a5c21112696fd165b
* Change-Id: 1235 (HEAD -> master) 8fee494b5170edb463fc623d03d562118cebe88e
* Change: foo7 cadc8f164b24465285d8ec413e0325a6341e4453
| * Change-Id: 1235 ef7c3c85ad4c5875f308003d42a6e11d9b14aeb9
| | * Change-Id: foo7 c1b55ea7e5f27f82d3565c1f5d64113adf635c2c
| | | * Change-Id: 1235 02796cbb12e05f3be9f16c82e4d26542af7e700c
| | | * Change-Id: foo7 2cbfa8cb8d9a9f1de029fcba547a6e56c742733f
| | * 9da166dcfa8650e04c7e39c54a61b7fa0b69ef4f
| | | * Change-Id: 1235 52310103e5d9a8e55df1a7766789a1af04d8601b
| | | * 48f307ad20210547fdf339d0b0d7ee02bc702c3d
| | |/
| |/|
| * | Change-Id: 1234 c61c37f4a3d5eb447f41dde15620eee1a181d60b
| |/
| * add file1 6ed6c1ca90cb15fe4edf8d133f0e2e44562aa77d
| * Change-Id: 1235 (origin/@changes/master/josh@example.com/1235) 96da92a9021ee186e1e9dd82305ddebfd1153ed5
|/
* Change-Id: 1234 (origin/@changes/master/josh@example.com/1234) 43d6fcc9e7a81452d7343c78c0102f76027717fb
| * Change-Id: foo7 (origin/@changes/master/josh@example.com/foo7) ecb19ea4b4fbfb6afff253ec719909e80a480a18
| * (origin/@changes/master/josh@example.com/foo7) 7a54645bd1415d8b911ea129f90fb962799846d2
| | * Change-Id: 1235 (origin/@heads/master/josh@example.com) 7fc3588b28a7c0e18be92f3e8303ccf632072804
| | * bc99e4e2bb4f77e86e65630057da2cea96110852
| |/
|/|
* | Change-Id: 1234 (origin/@changes/master/josh@example.com/1234) 43d6fcc9e7a81452d7343c78c0102f76027717fb
|/
* add file1 (origin/master, origin/HEAD) 5f2928c89c4dcc7f5a8c59ef65734a83620cefee
* Notes added by 'git_note_create' from libgit2 725a17751b9dc03b1696fb894d0643c5b6f0397d
Expand All @@ -150,9 +154,9 @@ Test normal push still works
$ git commit -q -m "add file4" -m "Change-Id: 1236"
$ josh push
To $TESTTMP/remote
6ed6c1c..84f0380 84f0380f63011c5432945683f8f79426cc6bd180 -> master
6ed6c1c..46af19d 46af19d75e628e41acb704f2fcae3973ed780d4a -> master

Pushed 84f0380f63011c5432945683f8f79426cc6bd180 to origin/master
Pushed 46af19d75e628e41acb704f2fcae3973ed780d4a to origin/master

Verify normal push worked

Expand Down