Junio C Hamano wrote: > Koichi Murase <myoga.murase@xxxxxxxxx> writes: > > > The raw command substitutions $v in the arguments of the test command > > and the [ command are subject to word splitting and pathname > > expansions. Even when it is ensured that the variable is not empty and > > does not contain whitespaces and glob characters, it can fail when IFS > > is set to non-trivial values containing letters and digits. > > The above sounded good before I looked at the patch, and it still is > good in theory, but it start to look mostly academic especially with > enclosing $# inside a pair of double-quotes, and the variable would > have only digits. The same for $i and $j that appear in the loop > control "for ((i=0, j=0; ...)); do". The story is pretty much the > same for local variables we set outselves to signal our findings, > like $pcmode that is only set to either 'yes' or 'no'. I do have the same opinion. Although the result seems more proper, I fail to see the actual value of doing all these changes everywhere. On the other hand I do see the very real harm that they would break the git-completion branch everywhere. Rebasing those 50-so patches would not be very pleasant. > In other words, this patch looks way too noisy to be reviewed to > discover its real worth. Agreed. -- Felipe Contreras
