Christian Couder <christian.couder@xxxxxxxxx> writes:
> +core.untrackedCache::
> + Determines if untracked cache will be automatically enabled or
> + disabled. It can be `keep`, `true` or `false`. Before setting
> + it to `true`, you should check that mtime is working properly
> + on your system.
> + See linkgit:git-update-index[1]. `keep` by default.
> +
Before "Before setting it to `true`", the reader needs to be told
what would happen when this is set to each of these three values (as
well as what happens when this is not set at all).
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index a0afe17..813f6cc 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -174,27 +174,31 @@ may not support it yet.
>
> --untracked-cache::
> --no-untracked-cache::
> - Enable or disable untracked cache extension. This could speed
> - up for commands that involve determining untracked files such
> - as `git status`. The underlying operating system and file
> - system must change `st_mtime` field of a directory if files
> - are added or deleted in that directory.
> + Enable or disable untracked cache extension. Please use
> + `--test-untracked-cache` before enabling it.
"extension" is a term that is fairly close to the machinery. In
other parts of the documentation (like the added text below in this
patch), it is called "untracked cache FEATURE", which probably is a
better word to use here as well.
> ++
> +These options cannot override the `core.untrackedCache` configuration
> +variable when it is set to `true` or `false` (see
> +linkgit:git-config[1]). They can override it only when it is unset or
> +set to `keep`. If you want untracked cache to persist, it is better
> +anyway to just set this configuration variable to `true` or `false`
> +directly.
While the above is not wrong per-se, from the point of those who
looked for these options (that is, those who wanted to do a one-shot
enabling or disabling of the feature, perhaps to try it out to see
how well it helps on their system), I think the explanation of the
interaction between the option and the config is backwards. For
their purpose, setting it to `true` or `false` will be hinderance,
because the options are made no-op by such a setting. IOW, the
advertisement "once you decided that you want to use the feature,
configuration is a better place to set it" does not belong here.
If I were writing this documentation, I'd probably rephrase the
second paragraph like so:
These options take effect only when the
`core.untrackedCache` configuration variable (see
linkgit:git-config[1]) is set to `keep` (or it is left
unset). When the configuration variable is set to `true`,
the untracked cache feature is always enabled (and when it
is set to `false`, it is always disabled), making these
options no-op.
perhaps.
> @@ -385,6 +389,34 @@ Although this bit looks similar to assume-unchanged bit, its goal is
> different from assume-unchanged bit's. Skip-worktree also takes
> precedence over assume-unchanged bit when both are set.
>
> +Untracked cache
> +---------------
> +
> +This cache could speed up commands that involve determining untracked
> +...
> +It is recommended to use the `core.untrackedCache` configuration
> +variable (see linkgit:git-config[1]) to enable or disable this feature
> +instead of using the `--[no-|force-]untracked-cache`, as it is more
> +persistant and visible with a configuration variable.
s/persistant/persistent/; but more importantly, I do not think
persistence has much to do with the choice between the option and
configuration. Once it is set with `--untracked-cache`, it should
persist until you explicitly use `--no-untracked-cache` to disable
it, no? Otherwise you have a bug that needs to be fixed.
The reason you'd want to use a configuration is because the effect
of using the `--untracked-cache` option is per repository (rather,
per index-file).
If you want to enable (or disable) this feature, it is easier to
use the `core.untrackedCache` configuration variable than using
the `--untracked-cache` option to `git update-index` in each
repository, especially if you want to do so across all
repositories you use, because you can set the configuration
variable to `true` (or `false`) in your `$HOME/.gitconfig` just
once and have it affect all repositories you touch.
or something, perhaps.
> +When the `core.untrackedCache` configuration variable is changed, the
> +untracked cache is added to or removed from the index the next time
> +"git status" is run; while when `--[no-|force-]untracked-cache` are
> +used, the untracked cache is immediately added to or removed from the
> +index.
Is it only "git status", or anything that writes/updates the index
file? The above makes it sound as if you are saying "If you change
the variable, you must run `git status` for the change to take
effect", and if that is indeed the case, perhaps you should say so
more explicitly. My cursory reading of the code suggests that the
user must run `git status` in a mode that shows untracked files
(i.e. "git status -uno" does not fix)?
I somehow thought, per the previous discussion with Duy, that the
check and addition of an empty one (if missing in the index and
configuration is set to `true`) or removal of an existing one (if
configuration is set to `false`) were to be done when the index is
read by read_index_from(), though. If you have to say "After
setting the configuration, you must run this command", that does not
sound like a huge improvement over "If you want to enable this
feature, you must run this command".
> + switch (untracked_cache) {
> + case UC_UNSPECIFIED:
> + break;
> + case UC_DISABLE:
> + if (use_untracked_cache == 1)
> + die("core.untrackedCache is set to true; "
> + "remove or change it, if you really want to "
> + "disable the untracked cache");
> remove_untracked_cache();
> report(_("Untracked cache disabled"));
> + break;
> + case UC_TEST:
> + setup_work_tree();
> + return !test_if_untracked_cache_is_supported();
> + case UC_ENABLE:
> + case UC_FORCE:
> + if (use_untracked_cache == 0)
> + die("core.untrackedCache is set to false; "
> + "remove or change it, if you really want to "
> + "enable the untracked cache");
> + add_untracked_cache();
> + report(_("Untracked cache enabled for '%s'"), get_git_work_tree());
> + break;
I do buy the decision to make these no-op when the configuration
says `true` or `false`, but I am not sure if these deserve die().
Exiting with 0 (= success) after issuing a warning() might be more
appropriate. I'd especially anticipate that some newbies will
complain that they got "fatal:" when they used the "--force-"
variant, saying "I know what I am doing, that is why I said 'force',
why does stupid Git refuse?".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html