Re: [PATCH v2 14/14] advice: update message to suggest '--sparse'

>>  	cat >sparse_error_header <<-EOF &&
>> -	The following pathspecs didn't match any eligible path, but they do match index
>> -	entries outside the current sparse checkout:
>> +	The following paths and/or pathspecs matched paths that exist outside of your
>> +	sparse-checkout definition, so will not be updated in the index:
>>  	EOF
>>  
>>  	cat >sparse_hint <<-EOF &&
>> -	hint: Disable or modify the sparsity rules if you intend to update such entries.
>> +	hint: Disable or modify the sparsity rules or use the --sparse option if you intend to update such entries.
>>  	hint: Disable this message with \"git config advice.updateSparsePath false\"
>>  	EOF
> 
> ...this used to line-wrap at 80 characters, but is now a bit beyond
> that.
> 
> Maybe instead make these two into bullet-points?

> This commentary pre-dates this commit, but just in general:
> 
> I think the advice system is best used where there's an initial
> non-optional message, and then the advice elaborates on what happened,
> how to fix it. A good example is the "short object ID %s is ambiguous"
> in object-name.c.
>
> But in this case both messages are rather long. I'd think better would
> be something like (and I didn't look very deeply at the involved code):
> 
>     error("pathspec '%s' matched only outside sparse checkout")
> 
> I.e. in e.g. cmd_rm() we loop through the pathspecs, and we error on the
> first one, so to a first approximation why do we need to for sparse emit
> ALL the pathspecs we didn't match? if we're going to error out anyway
> shouldn'w we just error out on the first one?

> But going on, I'd think this would be better overall (pseudocode):
> 
>     error("pathspec '%s' matched only outside sparse checkout")
>     if (advice_enabled(ADVICE_UPDATE_SPARSE_PATH)) {
>         char *list_str;
>         list_of_bad_pathspecs = make_that_list(my_pathspec_string_list, &list_str);
>   
>       if (list_of_bad_pathspecs.nr > 1)
>             /* Emit a message that details what's wrong, but also has a
>              * list of all the other pathspecs we'd also die on if the user */
>       else
>           /* Ditto, but no list *?

> Maybe I'm missing something with the sparse implemention, but I'd think
> going above & beyond and listing all failures is a bit much in either
> case, i.e. for non-sparse we have:
> 
>     $ git rm 'file-i-do-not-have' 'directory-i-do-not-have/'
>     fatal: pathspec 'file-i-do-not-have' did not match any files
>     $
> 
> I'd think in general a user who's screwed up and typo'd both isn't going
> to be much harmed by us noting the first, maybe they'll get another
> error then.
> 
> But usually it's obvious (e.g. you just ran the command in the wrong
> directory), so if you have a large list of pathspecs getting a firehose
> of all the things that didn't match can be less helpful due to being
> overrly verbose.