-
Notifications
You must be signed in to change notification settings - Fork 68
Implement :from filter #1519
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
Implement :from filter #1519
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,8 @@ filter_spec = { ( | |||||||
| filter_group | ||||||||
| | filter_message | ||||||||
| | filter_rev | ||||||||
| | filter_from | ||||||||
| | filter_concat | ||||||||
| | filter_join | ||||||||
| | filter_replace | ||||||||
| | filter_squash | ||||||||
|
|
@@ -53,6 +55,24 @@ filter_rev = { | |||||||
| ~ ")" | ||||||||
| } | ||||||||
|
|
||||||||
| filter_from = { | ||||||||
| CMD_START ~ "from" ~ "(" | ||||||||
| ~ NEWLINE* | ||||||||
| ~ (rev ~ filter_spec)? | ||||||||
| ~ (CMD_SEP+ ~ (rev ~ filter_spec))* | ||||||||
| ~ NEWLINE* | ||||||||
| ~ ")" | ||||||||
| } | ||||||||
|
|
||||||||
| filter_concat = { | ||||||||
| CMD_START ~ "concat" ~ "(" | ||||||||
|
||||||||
| ~ NEWLINE* | ||||||||
| ~ (rev ~ filter_spec)? | ||||||||
| ~ (CMD_SEP+ ~ (rev ~ filter_spec))* | ||||||||
|
Comment on lines
+70
to
+71
|
||||||||
| ~ (rev ~ filter_spec)? | |
| ~ (CMD_SEP+ ~ (rev ~ filter_spec))* | |
| ~ rev ~ filter_spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to keep it like that for now to make it easier to see the similarity with other rules.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -317,6 +317,8 @@ enum Op { | |||||
| Pattern(String), | ||||||
| Message(String, regex::Regex), | ||||||
|
|
||||||
| HistoryConcat(LazyRef, Filter), | ||||||
|
|
||||||
| Compose(Vec<Filter>), | ||||||
| Chain(Filter, Filter), | ||||||
| Subtract(Filter, Filter), | ||||||
|
|
@@ -441,6 +443,14 @@ fn lazy_refs2(op: &Op) -> Vec<String> { | |||||
| av | ||||||
| } | ||||||
| Op::Rev(filters) => lazy_refs2(&Op::Join(filters.clone())), | ||||||
| Op::HistoryConcat(r, f) => { | ||||||
| let mut lr = Vec::new(); | ||||||
| if let LazyRef::Lazy(s) = r { | ||||||
| lr.push(s.to_owned()); | ||||||
| } | ||||||
| lr.append(&mut lazy_refs(*f)); | ||||||
| lr | ||||||
| } | ||||||
| Op::Join(filters) => { | ||||||
| let mut lr = lazy_refs2(&Op::Compose(filters.values().copied().collect())); | ||||||
| lr.extend(filters.keys().filter_map(|x| { | ||||||
|
|
@@ -501,6 +511,19 @@ fn resolve_refs2(refs: &std::collections::HashMap<String, git2::Oid>, op: &Op) - | |||||
| .collect(); | ||||||
| Op::Rev(lr) | ||||||
| } | ||||||
| Op::HistoryConcat(r, filter) => { | ||||||
| let f = resolve_refs(refs, *filter); | ||||||
| let resolved_ref = if let LazyRef::Lazy(s) = r { | ||||||
| if let Some(res) = refs.get(s) { | ||||||
| LazyRef::Resolved(*res) | ||||||
| } else { | ||||||
| r.clone() | ||||||
| } | ||||||
| } else { | ||||||
| r.clone() | ||||||
| }; | ||||||
| Op::HistoryConcat(resolved_ref, f) | ||||||
| } | ||||||
| Op::Join(filters) => { | ||||||
| let lr = filters | ||||||
| .iter() | ||||||
|
|
@@ -658,6 +681,9 @@ fn spec2(op: &Op) -> String { | |||||
| Op::Message(m, r) => { | ||||||
| format!(":{};{}", parse::quote(m), parse::quote(r.as_str())) | ||||||
| } | ||||||
| Op::HistoryConcat(r, filter) => { | ||||||
| format!(":concat({}{})", r.to_string(), spec(*filter)) | ||||||
|
||||||
| format!(":concat({}{})", r.to_string(), spec(*filter)) | |
| format!(":concat({}:{})", r.to_string(), spec(*filter)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true. the : is part of the spec.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| $ export TESTTMP=${PWD} | ||
|
|
||
| $ cd ${TESTTMP} | ||
| $ git init -q libs 1> /dev/null | ||
| $ cd libs | ||
|
|
||
| $ mkdir sub1 | ||
| $ echo contents1 > sub1/file1 | ||
| $ git add sub1 | ||
| $ git commit -m "add file1" 1> /dev/null | ||
|
|
||
| $ echo contents2 > sub1/file2 | ||
| $ git add sub1 | ||
| $ git commit -m "add file2" 1> /dev/null | ||
| $ git update-ref refs/heads/from_here HEAD | ||
|
|
||
|
|
||
| $ mkdir sub2 | ||
| $ echo contents1 > sub2/file3 | ||
| $ git add sub2 | ||
| $ git commit -m "add file3" 1> /dev/null | ||
|
|
||
| $ josh-filter ":\"x\"" | ||
|
|
||
| $ git log --graph --pretty=%s:%H HEAD | ||
| * add file3:667a912db7482f3c8023082c9b4c7b267792633a | ||
| * add file2:81b10fb4984d20142cd275b89c91c346e536876a | ||
| * add file1:bb282e9cdc1b972fffd08fd21eead43bc0c83cb8 | ||
|
|
||
| $ git log --graph --pretty=%s:%H FILTERED_HEAD | ||
| * x:9d117d96dfdba145df43ebe37d9e526acac4b17c | ||
| * x:b232aa8eefaadfb5e38b3ad7355118aa59fb651e | ||
| * x:6b4d1f87c2be08f7d0f9d40b6679aab612e259b1 | ||
|
|
||
| $ josh-filter -p ":from(81b10fb4984d20142cd275b89c91c346e536876a:\"x\")" | ||
| :"x":concat(81b10fb4984d20142cd275b89c91c346e536876a:"x") | ||
| $ josh-filter ":from(81b10fb4984d20142cd275b89c91c346e536876a:\"x\")" | ||
|
|
||
| $ git log --graph --pretty=%s FILTERED_HEAD | ||
| * x | ||
| * add file2 | ||
| * add file1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The grammar for
filter_fromis more permissive than the implementation. The grammar allows optional arguments and multiple comma-separated pairs with(rev ~ filter_spec)? ~ (CMD_SEP+ ~ (rev ~ filter_spec))*, but the parsing code (lines 184-193 in parse.rs) requires exactly 2 elements. The grammar should enforce exactly one rev and one filter_spec by usingrev ~ filter_specwithout the optional?and without the repetition*.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to combine all those similar rules into one eventually so I'd like to keep it the same as the others.