{"thread":{"id":"28938","subject":"[PATCH] revert: prettify fatal messages","startedAt":"2011-11-15T09:31:32Z","lastAt":"2011-11-16T06:42:12Z","messageCount":5,"participants":["Ramkumar Ramachandra","Matthieu Moy","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"179491","messageId":"1321349492-5653-1-git-send-email-artagnon@gmail.com","threadId":"28938","inReplyTo":null,"subject":"[PATCH] revert: prettify fatal messages","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-15T09:31:32Z","receivedAt":"2011-11-15T09:31:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Some of the fatal messages printed by revert and cherry-pick look ugly\nlike the following:\n\n fatal: Could not open .git/sequencer/todo.: No such file or directory\n\nThe culprit here is the die_errno() function that takes a custom error\nmessage string as an argument and appends \": \"\nbefore printing it. So, we can fix the problem by not terminating the\nstring argument to the function with a '.' (period). Fatal messages\nlook nicer now:\n\n fatal: Could not open .git/sequencer/todo: No such file or directory\n\nSigned-off-by: Ramkumar Ramachandra \n---\n Candidate for 'maint'?\n\n builtin/revert.c | 10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 87df70e..e0319c8 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -331,7 +331,7 @@ static void write_message(struct strbuf *msgbuf, const char *filename)\n \tint msg_fd = hold_lock_file_for_update(&msg_file, filename,\n \t\t\t\t\t LOCK_DIE_ON_ERROR);\n \tif (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)\n-\t\tdie_errno(_(\"Could not write to %s.\"), filename);\n+\t\tdie_errno(_(\"Could not write to %s\"), filename);\n \tstrbuf_release(msgbuf);\n \tif (commit_lock_file(&msg_file) < 0)\n \t\tdie(_(\"Error wrapping up %s\"), filename);\n@@ -767,7 +767,7 @@ static void read_populate_todo(struct commit_list **todo_list,\n \n \tfd = open(todo_file, O_RDONLY);\n \tif (fd < 0)\n-\t\tdie_errno(_(\"Could not open %s.\"), todo_file);\n+\t\tdie_errno(_(\"Could not open %s\"), todo_file);\n \tif (strbuf_read(&buf, fd, 0) < 0) {\n \t\tclose(fd);\n \t\tstrbuf_release(&buf);\n@@ -845,7 +845,7 @@ static int create_seq_dir(void)\n \tif (file_exists(seq_dir))\n \t\treturn error(_(\"%s already exists.\"), seq_dir);\n \telse if (mkdir(seq_dir, 0777) < 0)\n-\t\tdie_errno(_(\"Could not create sequencer directory '%s'.\"), seq_dir);\n+\t\tdie_errno(_(\"Could not create sequencer directory %s\"), seq_dir);\n \treturn 0;\n }\n \n@@ -859,7 +859,7 @@ static void save_head(const char *head)\n \tfd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);\n \tstrbuf_addf(&buf, \"%s\\n\", head);\n \tif (write_in_full(fd, buf.buf, buf.len) < 0)\n-\t\tdie_errno(_(\"Could not write to %s.\"), head_file);\n+\t\tdie_errno(_(\"Could not write to %s\"), head_file);\n \tif (commit_lock_file(&head_lock) < 0)\n \t\tdie(_(\"Error wrapping up %s.\"), head_file);\n }\n@@ -876,7 +876,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n \t\tdie(_(\"Could not format %s.\"), todo_file);\n \tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n \t\tstrbuf_release(&buf);\n-\t\tdie_errno(_(\"Could not write to %s.\"), todo_file);\n+\t\tdie_errno(_(\"Could not write to %s\"), todo_file);\n \t}\n \tif (commit_lock_file(&todo_lock) < 0) {\n \t\tstrbuf_release(&buf);\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"179494","messageId":"vpqlirhitvw.fsf@bauges.imag.fr","threadId":"28938","inReplyTo":"1321349492-5653-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] revert: prettify fatal messages","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-11-15T10:04:35Z","receivedAt":"2011-11-15T10:04:35Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra writes:\n\n> Some of the fatal messages printed by revert and cherry-pick look ugly\n> like the following:\n>\n> fatal: Could not open .git/sequencer/todo.: No such file or directory\n>\n> The culprit here is the die_errno() function that takes a custom error\n> message string as an argument and appends \": \"\n> before printing it. So, we can fix the problem by not terminating the\n> string argument to the function with a '.' (period). Fatal messages\n> look nicer now:\n>\n> fatal: Could not open .git/sequencer/todo: No such file or directory\n\nSounds (obviously) good.\n\n(I just misread the subject line at first, and understood that this was\na revert of an earlier commit, while this is actually a commit touching\nthe \"revert\" part of Git)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"179545","messageId":"7vvcqkswpu.fsf@alter.siamese.dyndns.org","threadId":"28938","inReplyTo":"1321349492-5653-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] revert: prettify fatal messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-16T01:05:33Z","receivedAt":"2011-11-16T01:05:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Candidate for 'maint'?\n\nToo minor to matter when 'master' is already at -rc2.\n\nThanks.\n"},{"id":"179553","messageId":"CALkWK0m+us0rCx61+SwSsKuO7qK5mbCs0MpoAzB4ja=wBA+Xjw@mail.gmail.com","threadId":"28938","inReplyTo":"7vvcqkswpu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] revert: prettify fatal messages","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-16T06:23:19Z","receivedAt":"2011-11-16T06:23:19Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nMatthieu Moy wrote:\n> (I just misread the subject line at first, and understood that this was> a revert of an earlier commit, while this is actually a commit touching> the \"revert\" part of Git)\nReminds me of an earlier note Jonathan made about the git-revert\ncommit message format. To avoid confusion, reverts should always look\nlike the following:\n\n Revert [commit message]\n\n This reverts commit [unabbreviated sha1 hex].\n\nJunio C Hamano wrote:\n> Ramkumar Ramachandra writes:\n>>  Candidate for 'maint'?\n>\n> Too minor to matter when 'master' is already at -rc2.\n\nAh, I haven't been paying much attention to these release cycles. I\nsuppose we can put this patch in `master` after the release? I wince\neverytime I see these ugly fatal messages: it happens shockingly often\nwhile testing :)\n\nThanks.\n\n-- Ram\n"},{"id":"179555","messageId":"20111116064013.GA13706@elie.hsd1.il.comcast.net","threadId":"28938","inReplyTo":"CALkWK0m+us0rCx61+SwSsKuO7qK5mbCs0MpoAzB4ja=wBA+Xjw@mail.gmail.com","subject":"Re: [PATCH] revert: prettify fatal messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-16T06:42:12Z","receivedAt":"2011-11-16T06:42:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> To avoid confusion, reverts should always look\n> like the following:\n>\n> Revert [commit message]\n>\n> This reverts commit [unabbreviated sha1 hex].\n\nDon't forget the description of why it is being reverted. :)\n\nActually, I think Matthieu's confusion makes a lot of sense. I don't\nthink it matters very much, but it can sometimes be clearer to say\n\"cherry-pick/revert: remove spurious period in error message\".\n\nAnother benefit to moving the juicy part to sequencer.c, I guess.\n"}]}