Subject: Re: [PATCH v4 1/8] fast-import: tighten path unquoting

From: Junio C Hamano <[email-masked]>
Date: Sat, 13 Apr 2024 11:33:02 -0700

Thalia Archibald <[email-masked]> writes:

> As to naming, `allow_spaces` and `include_spaces` are problematic for
> the reasons you both have pointed out. I think `stop_at_unquoted_space`
> is problematic, because that’s not where it stops when quoted, but
> rather at the close quote. I think that `include_unquoted_spaces` is
> good, because it describes that spaces are included in this field when
> it is an unquoted string. `allow_unquoted_spaces` implies that its an
> error to have a space, but no such error is raised here.

OK, so the bit tells the function if we are dealing with the last
field on the input line, because unquoted side needs to know when
to stop reading the path.

	static void parse_path(... int is_last_field ...)
	{
		if (*p == '"') {
			... handling of a quoted path is unchanged
			... regardless of where on the line it apears
		} else {
			/*
			 * unless we know this is the last field,
			 * an unquoted SP is the end of this field.
			 */
			*endp = is_last_field 
                              ? p + strlen(p)
			      : strchrnul(p, ' ');
		}
	}

Another way to look at it is that we are telling the function if a
space in an unquoted path is part of the path or not.

The distinction matters only if we require, in some record type, a
path field that is the last on the line to be quoted when it has a
SP in it, in which case, "is_last_field" is a wrong name, and we
need to say something like space_is_end_of_field_if_unquoted (or we
can reverse the polarity to say unquoted_space_is_part_of_the_path,
include_unqouted_space, etc.).  But if not, I find that "we normally
stop at SP when unquoted but the last field is a special case"
somewhat more natural.  I do not feel too strongly, though.

Thanks.