On 20/08/2019 04:05, brian m. carlson wrote:
On 2019-08-19 at 09:55:27, Phillip Wood wrote:
On 19/08/2019 10:41, Phillip Wood wrote:
[...]
diff --git a/convert.c b/convert.c
index 94ff837649..030e9b81b9 100644
--- a/convert.c
+++ b/convert.c
@@ -1293,10 +1293,11 @@ struct conv_attrs {
const char *working_tree_encoding; /* Supported encoding or
default encoding if NULL */
};
+static struct attr_check *check;
I was concerned about the impact adding a file global if we ever want to
multi-thread this for submodules, but looking through the file there are
a couple of others already so this isn't creating a new problem.
Doh, I've just realized it was static already - ignore that.
And I just realized that I didn't read the entire thread before
responding. Sorry about that.
One thing did occur to me though - does this patch reset attributes like the
merge marker length (they're less critical though if there is a conflict
after an attribute change it would be nice to have the correct length) or
just the ones for filtering files?
It resets "crlf", "ident", "filter", "eol", "text", and
"working-tree-encoding". Things it doesn't reset include "whitespace",
"export-ignore", "export-subst", "merge", and "conflict-marker-size".
Of these, I think only the latter two are relevant.
I'll update that in v5.
Thanks, one other thought I had was that this is really a bug in 'am'
rather than 'rebase'. There has been some talk of switching the default
rabase backend to the sequencer so maybe it would be better to test 'am'
rather than 'rebase' to ensure we still check 'am' is working properly
after any switch in the rebase backend. (perhaps we should be testing
rebase and rebase --interactive separately as well to prevent any future
regressions I'm not sure)
Best Wishes
Phillip
