Hi,
Your code is not C90:
wt-status.c: In function ‘get_two_last_lines’:
wt-status.c:1030:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
struct strbuf buf = STRBUF_INIT;
^
wt-status.c: In function ‘get_two_first_lines’:
wt-status.c:1050:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
struct strbuf buf = STRBUF_INIT;
^
I have this to notice it:
$ cat config.mak
CFLAGS += -Wdeclaration-after-statement -Wall -Werror #-O0 -g
Guillaume Pagès <guillaume.pages@xxxxxxxxxxxxxxxxxxxxxxx> writes:
> +void get_two_last_lines(char *filename,int * numlines, char ** lines)
Space after ,
> +{
> + *numlines=0;
Spaces around =
> + struct strbuf buf = STRBUF_INIT;
> + FILE *fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n')!=EOF){
Spaces around !=
There are many other space issues, I won't list them all.
> + (*numlines)++;
> + lines[0]=lines[1];
> + lines[1]=strbuf_detach(&buf, NULL);
> + }
> + if (!fclose(fp))
> + strbuf_detach(&buf, NULL);
> + else
> + strbuf_release(&buf);
> +}
> +
> +void get_two_first_lines(char *filename,int * numlines, char ** lines)
Stick the * to the corresponding variable.
Do: int *numlines
Don't: int * numlines, int* numlines
One rationale is that when you write
int *i, j;
it reads "*i is an int, and so is j" (which is right), while
int * i, j;
may read as "both i and j are int *" (which is not right).
> +{
> + *numlines=0;
> + struct strbuf buf = STRBUF_INIT;
> + char *line;
> + FILE *fp = fopen(git_path("%s", filename), "r");
> + if (!fp) {
> + strbuf_release(&buf);
> + return;
> + }
> + while (strbuf_getline(&buf, fp, '\n')!=EOF){
> + stripspace(&buf, 1);
> + line = strbuf_detach(&buf, NULL);
> + if (strcmp(line,"")==0)
> + continue;
> + if (*numlines<2)
> + lines[*numlines] = line;
> + (*numlines)++;
> + }
> + if (!fclose(fp))
> + strbuf_detach(&buf, NULL);
> + else
> + strbuf_release(&buf);
> +}
> +
> +void show_rebase_information(struct wt_status *s,
> + struct wt_status_state *state,
> + const char *color)
static?
> +{
> + if (state->rebase_interactive_in_progress){
> + int i, begin;
> + int numlines =0;
> + char* lines[2];
> + get_two_last_lines("rebase-merge/done", &numlines, lines);
> + if (numlines==0)
> + status_printf_ln(s,color,"No command done.");
> + else{
Space before {. Several instances too.
> + status_printf_ln(s,color,
> + "Last command(s) done (%d command(s) done):",
> + numlines);
> + begin = numlines > 1? 0 : 1;
> + for (i = begin; i < 2; i++){
> + status_printf_ln(s,color," %s",lines[i]);
> + }
> + if (numlines>2 && s->hints )
> + status_printf_ln(s,color,
> + " (see more at .git/rebase-merge/done)");
> + }
> + numlines =0;
> + get_two_first_lines("rebase-merge/git-rebase-todo",
> + &numlines, lines);
> + if (numlines==0)
> + status_printf_ln(s,color,
Space after ,.
> + "No command remaining.");
> + else{
> +
> + status_printf_ln(s,color,
> + "Next command(s) to do (%d remaining command(s)):",
> + numlines);
> + begin = numlines > 1? 0 : 1;
> + for (i = 0; (i < 2 && i < numlines); i++){
> + status_printf(s,color," %s",lines[i]);
> + }
> + if (s->hints )
No space before ).
> + status_printf_ln(s,color,
> + " (use git rebase --edit-todo to view and edit)");
Mark for translation -> _("..."). Many instances above.
> - if (has_unmerged(s)) {
> + show_rebase_information(s,state,color);
> + if (has_unmerged(s) || state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
The show_rebase_information() line does belong to this patch, but the
other change do not IHMO. It would be _much_ easier to review in its own
patch with an explicit title like "status: factor two rebase-related
messages together" or so.
> @@ -1327,7 +1410,10 @@ void wt_status_print(struct wt_status *s)
> else if (!strcmp(branch_name, "HEAD")) {
> branch_status_color = color(WT_STATUS_NOBRANCH, s);
> if (state.rebase_in_progress || state.rebase_interactive_in_progress) {
> - on_what = _("rebase in progress; onto ");
> + if (state.rebase_interactive_in_progress)
> + on_what = _("interactive rebase in progress; onto ");
> + else
> + on_what = _("rebase in progress; onto ");
I wouldn't insist on that, but splitting this one in a separate patch
would make sense to me too. The patch would contain this change, and the
impact on tests.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html