Re: [PATCH v2 5/5] last-modified: change default max-depth to 0
From: Junio C Hamano <hidden>
Date: 2026-01-16 18:55:24
Toon Claes [off-list ref] writes:
quoted hunk
diff --git a/Documentation/git-last-modified.adoc b/Documentation/git-last-modified.adoc index a3992db3f2..57136baf3b 100644 --- a/Documentation/git-last-modified.adoc +++ b/Documentation/git-last-modified.adoc@@ -27,6 +27,7 @@ OPTIONS `--recursive`:: Instead of showing tree entries, step into subtrees and show all entries inside them recursively. + This is identical as setting `--max-depth=-1`.
When I heard that the default value of max-depth will be 0, the first thing I wondered was "how would I spell unlimited in the new world order?", and the documentation for "--max-depth", not "--recursive", would have been the place I expected to fish for necessary information. Over there, there is "A negative value means no limit", so saying "identical as setting --max-depth to a negative value" here would match the description over there better, or the user will be left wonderign if "-1" is merely an example that is negative, or if it is more special than other negative values and if so in what way.
quoted hunk
@@ -36,7 +37,7 @@ OPTIONS `--max-depth=<depth>`:: For each pathspec given on the command line, descend at most `<depth>` levels of directories. A negative value means no limit. - Setting a positive value implies `--recursive`. + The default depth is 0. Cannot be combined with wildcards in the pathspec.
quoted hunk
diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 842700bc6a..a10e711beb 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c@@ -481,14 +481,10 @@ static int last_modified_init(struct last_modified *lm, struct repository *r, lm->rev.no_commit_id = 1; lm->rev.diff = 1; lm->rev.diffopt.flags.no_recursive_diff_tree_combined = 1; - lm->rev.diffopt.flags.recursive = lm->recursive; + lm->rev.diffopt.flags.recursive = 1;
Hmph, so this will always be recursive?
lm->rev.diffopt.flags.tree_in_recursive = lm->show_trees; + lm->rev.diffopt.max_depth = lm->max_depth; + lm->rev.diffopt.max_depth_valid = !lm->recursive && lm->max_depth >= 0;
Not saying --recursive would keep lm->recursive==0 and non-negative value of --max-depth will flip max_depth_valid on. Saying "--recursive" or giving a negative "--max-value" would make max_depth_valid false, and it allows traversal all the way down to leaves. It may be correct, but feels quite convoluted. I wonder if we can get rid of lm->recursive altogether now as a clean-up, and have "--recursive" truly do what the documentation claimed is equivalent earlier, i.e. OPT_SET_INT(0, "recursive", &lm.max_depth, -1). Would that simplify the logic a bit and make it easier to reason about the logic around here, I wonder?