On 10/21/2019 11:04 AM, Phillip Wood wrote: > Hi Stolee > > On 21/10/2019 14:56, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> >> The read-tree builtin has a --verbose option that signals to show >> progress and other data while updating the index. Update this to >> be on by default when stderr is a terminal window. >> >> This will help tools like 'git sparse-checkout' to automatically >> benefit from progress indicators when a user runs these commands. >> >> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> --- >> builtin/read-tree.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/builtin/read-tree.c b/builtin/read-tree.c >> index ca5e655d2f..69963d83dc 100644 >> --- a/builtin/read-tree.c >> +++ b/builtin/read-tree.c >> @@ -162,6 +162,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) >> opts.head_idx = -1; >> opts.src_index = &the_index; >> opts.dst_index = &the_index; >> + opts.verbose_update = isatty(2); > > I'm slightly wary of changing the output of plumbing commands like this. If a script wants progress output it can already get it by passing --verbose. With this change a script that does not want that output now has to pass --no-verbose. If a script is calling this, then won't stderr not be a terminal window, and isatty(2) return 0? Or, if the script is run with stderr passing through to a terminal, then the user would see progress while running the script, which seems like a side-effect but not one that will cause a broken script. > If 'git sparse-checkout' wants progress indicators why not put the isatty() check there and pass the appropriate option to read-tree? It is not necessary for the sparse-checkout builtin by the end of the series, as the extra process is removed and unpack_trees() is called directly. However, it does seem like a helpful user-interaction for people still using the "old" way of interacting with the sparse-checkout feature. Thanks, -Stolee
