Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: >> I think my puzzlement comes from here. What makes it OK for "am" to >> expect the contents of author-script file to be quoted but it is not >> OK to expect the same here? What makes it not quoted for _this_ >> reader, in other words? > > The `git am` command is inherently *not* interactive, while the > interactive rebase, well, is. > > As such, we must assume that enterprisey users did come up with scripts > that edit, or create, author-script files, and exploited the fact that the > interactive rebase previously sourced them. It's like saying "While the program is waiting for user interaction, the user can attach a debugger to tweak the contents of this string variable. Hence we must not assume that the string is always NUL terminated to protect ourselves." A correct response to such a user is to tell them not to do that, and do so at two levels. The first level may be "it may be physically possible to tweak the string via debugger but don't do that in such a way that breaks the invariants our program relies on, or you are on your own", but more important is the response at the second level. Why is the user futzing with that string in the first place with the debugger? In the context of author-script, the question is "Why is the user futzing with the author-script file"? To attribute the resulting commit to a different author, of course. But we need to step back and think why the user needs to resort to editing that file to achieve that. If that is a common thing users would want to do, then we should offer an official way to do so, and it should not be "you can futz with author-script file with your editor". Something like "have 'exec git commit --amend' in the todo file" may be more appropriate and if it is important enough, the sequencer command language may want to learn an extra verb to update the author. Besides, the opportunity easiest for the user to futz with the contents of author-script file (or "attach the debugger while we wait for user interaction") arises when "rebase -i" or "am" stops waiting for conflict resolution. Even when you run "am" without its "-i" option, it is equally susceptible to the "user futzing with the file", which means your "am is not interactive but 'rebase -i' is" is irrelevant to the issue. Oh, also, did I say "am" has "-i" option, which is a short-hand for "--interactive"? What disturbs me the most is that I know you know the system well enough to realize how bogus your argument to claim that "rebase -i" and "am" are different was and to come up with what I wrote above yourself. Which means that I need to conclude that you have some other reasons why you want to keep this parser different, but I still do not know what they are. I guess it entirely is possible that one of the reasons is because some later patches in the larger "rebase-i to sequencer" series writes author-script file in a syntax that cannot be read by the recently refactored code "am" uses to read the author-script file, and reusing the existing code may end up breaking the endgame "rebase -i" you have. As I do not feel like arguing with you on this any longer, and as I certainly do not want to be blamed for breaking your "rebase -i", I do not insist the code to be refactored to share with the existing codepath. But still I do not see the need to keep them separate (as I already said in the previous message, I am OK if the one used in "am" is updated to match).