Re: [PATCH] Implement git commit as a builtin.

From: Carlos Rica <jasampler@gmail.com>
Date: Jul 18, 2007, 21:27 UTC
Message-ID: <1b46aba20707181427y12bd8b62pe30df61219e7c1f7@mail.gmail.com>
In-Reply-To: <11847863792344-git-send-email-krh@redhat.com>

2007/7/18, Kristian Høgsberg <krh@redhat.com>:

Show 7 quoted lines

> +       if (buffer) {
> +               len = strip_lines(buffer, len);
> +
> +               if (fwrite(buffer, 1, len, fp) < len)
> +                       die("could not write commit template: %s\n",
> +                           strerror(errno));
> +       }

....

Show 5 quoted lines

> +       len = strip_lines(buffer, len);
> +
> +       if (message_is_empty(buffer, len))
> +               die("* no commit message?  aborting commit.");
> +

Hi Kristian, you could call to the new stripspace() function in builtin-stripspace.c, to reduce code in your file. The only thing you should consider is that the new stripspace() removes always the last '\n' in the file (if any), so you have to add it when you need.

I sent a patch to change the name of read_pipe with read_fd (not accepted yet), and to terminate the buffer with NUL (although perhaps you don't need that), and to allow the function to receive buffer NULL or size 0. Your version was a lot different, therefore you would need to set buffer and size before calling it and free the buffer if reading failed after that: http://article.gmane.org/gmane.comp.version-control.git/52835

Now I need to create the new file editor.c to group at least three functions related with reading and editing text. Those also will reduce the size of your code and also can be reused from builtin-tag.c: launch_editor, read_path (as read_file someone said), and stripspace.

The first problem now should be how to write a version for launch_editor() which both builtins (yours and mine) could share, because the references in your launch_editor() related to commits, removed already in my version for git-tag.

I did this job trying to make easier your porting by moving those parts shared with builtin-tag.c outside your code and generating discussion on them, so you can concentrate just in the specific parts and reuse the rest. I hope those can help you.

--- Carlos Rica