Re: [RFC PATCH 0/1] config: surface editor failure in exit code

Karthik Nayak <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <CAOLa=ZTykwSDcFaEmEJJ1PTnX5L9=2t+tkCWhF+hV4J9EPBwWg@mail.gmail.com>
Junio C Hamano <[email protected]> writes:

> Kenneth Lorber <[email protected]> writes:
>
>> When the editor invoked by 'git config -e' fails (crashes or calls exit(3)
>> with a non-zero value), git notices and give an error:
>> 	editor.c:launch_specified_editor()
>> 		return error("there was a problem with the editor '%s'", editor);
>> which is then lost:
>> 	builtin/config.c:show_editor()
>> 		launch_editor(config_file, NULL, NULL);
>> which results in git always calling exit(0).  Note that the value is
>> not explicitly thrown away with "(void)", so this may not have been
>> intentional.
>
> I do not intentionally exit my editor with a non-zero status myself,
> but what I hear from others who do is that they do so to affect the
> invoking 'git' command, e.g., to stop 'git commit' from creating a
> commit.  They somehow realize they botched the edit, and they want
> to prevent 'git commit' from committing, signaling that by exiting
> their editor.  A cleaner and more modern way to do so, by the way,
> is to empty the editor buffer.  In either case, 'git commit' itself
> exits with a non-zero status.
>
> It might have been more consistent if 'git config -e' exited with a
> non-zero status when it noticed that the editor exited with a
> non-zero status, in that sense.  But we have never done so, and that
> is probably because we did not care ;-)
>
> In any case, I am not sure whether there is much value in making
> 'git config -e' start behaving that way.  Even if it can notice a
> failed editor, the damage to the file is already done, and there is
> not enough information to undo the damage even if you wanted to when
> detecting such an error.  This is quite different from when an editor
> edits the 'COMMIT_EDITMSG' file and fails.
>
> So, I dunno.

Wouldn't it be better to notify the user that something went wrong
rather than simply brush it off?

I would be in support of the patch:

  $ GIT_EDITOR=false git config --edit
  error: there was a problem with the editor 'false'
  $ echo $status
  0

As a user the expectation here would be a non-zero exit status.
signature.asc (application/pgp-signature, 690 B)
-----BEGIN PGP SIGNATURE-----

iQHKBAEBCgA0FiEEV85Mf2N1cQ/LZcYGPtWfJI5GjH8FAmqEFzoWHGthcnRoaWsu
MTg4QGdtYWlsLmNvbQAKCRA+1Z8kjkaMfzMBC/4z1qwEWg4F21CkgOvmTdW3yiDj
/NA/+iTn34/OenuxbtN8msms/idR03IjXozsiezty4HJxSg+tQ+pDlBPvKMHBnvQ
DAZdaLa+EAghdJ0EeCf93GW9NDgZMAXq8J6wOTCzsMthDHuPqn1V740uwSzN9WjB
yrmMItfBzrEHNpagWVgZIG14cigwbh+4qNUOGi/NvOIyPD8WtcL1/J3/l8OIJMoB
YCRkW3lKvxdMPESXwvlq1RbQ/G9MgPC5Ox9CBBXWL34SdOVFK/tHkEG7gnrc9ukg
oOpaGo1toFvrSgi0qa9LLP4iQw6uCjigqDSQj18EiS7TnPA2WtGbzMSiuEZjSV4q
ESMkICQ/mddKAdoZTxx+CNTrClPaaAwtF8ErvLjfjH8wUxXUoyGgRt/1Q9QYwOXg
kcJz4x0xCjBrzS7GHAF3ciFhk0U0hbvf/06yxgyXpwbTBlzBI6z7HHjvlHAQP3oT
2+HB5ux8pXMIY99PQZyfKEMLYjyjN9vsVco/9Ks=
=H0ma
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.