Junio C Hamano <gits...@pobox.com> writes:
> Johannes Sixt <j...@kdbg.org> writes:
>
>> On Windows, 'git rebase -i' with rebase.missingCommitsCheck set to
>> warn or error reports:
>>
>> Dropped commits (newer to older):
>> 'atal: bad revision '410dee56...
>>
>> The error comes from the git rev-list --stdin invocation in
>> git-rebase--interactive.sh (function check_todo_list)....
We have other places that take long list of things via --stdin
option. It somehow feels incomplete to patch only rev-list and not
others, doesn't it?
I looked at hits from 'grep -e --stdin Documentation/'. Here are
the findings.
1. These use strbuf_getline() to get one line at a time into a
strbuf and expects the line termination stripped off (i.e. these
callers do not want to worry about having LF at the end):
check-attr --stdin
check-ingore --stdin
check-mailmap --stdin
checkout-index --stdin
hash-object --stdin-paths
http-fetch --stdin
notes --stdin
send-pack --stdin
update-index --index-info
2. Any command in the "log" family uses strbuf_getwholeline(), so it
needs to know about the LF at the end explicitly:
rev-list --stdin
show --stdin
cherry-pick --stdin
...
3. This uses fgets() into a fixed buffer; it calls get_sha1_hex() on
it, and the expected input is one 40-hex per line, so it does not
matter if there is an extra CR at the end immediately before LF.
diff-tree --stdin
4. This slurps everything in-core, instead of going line-by-line.
update-ref --stdin
Now, I am wondering if it makes sense to do these two things:
* Teach revision.c::read_revisions_from_stdin() to use
strbuf_getline() instead of strbuf_getwholeline().
* Teach strbuf_getline() to remove CR at the end when stripping the
LF at the end, only if "term" parameter is set to LF.
Doing so would solve 1. and 2., but we obviously need to audit all
the other uses of strbuf_getline() to see if they can benefit (or if
some of them may be broken because they _always_ need LF terminated
lines, i.e. CRLF terminated input is illegal to them).
As to 3., I think it is OK. The code structure of 4. is too ugly
and needs to be revamped to go one line at a time first before even
thinking about how to proceed, I would think.
Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
