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-----