Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:46

Junio C Hamano [off-list ref] writes:
The above is most likely what I would have written if I were doing
this patch.  I could squash it to save a round-trip, but let me run
the testsuite first to see if we need adjustments to existing tests.
Strangely running t4201 with your patch (without any squashing)
seems to show a breakage in shortlog.  I won't be able to come back
to this topic for at least a few hours, so this is just a single bit
"breaks" report, without "how and why" analysis, sorry.
quoted hunk
Also your idea:
quoted
But the code *could* be made to just always do the whole
"strbuf_add()", and not return a return value at all, and the no-tab
case wouldn't be explicitly written to be different.
may give us a better structure if we are going to give users a knob
to disable this tab expansion, i.e. move the addition of 4 spaces to
the caller, name the body of such a function strbuf_expand_add(),
and then make the caller do something like this perhaps?
@@ -1723,10 +1711,14 @@ void pp_remainder(struct pretty_print_context *pp,
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent) {
-			if (pp_handle_indent(sb, indent, line, linelen))
-				linelen = 0;
+			strbuf_addchars(sb, ' ', indent);
+			if (pp->fmt == CMIT_FMT_EXPAND_TABS)
+				strbuf_expand_add(sb, line, linelen);
+			else
+				strbuf_add(sb, line, linelen);
+		} else {
+			strbuf_add(sb, line, linelen);
 		}
-		strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 23:08:46

On Wed, Mar 16, 2016 at 12:47 PM, Junio C Hamano [off-list ref] wrote:
Strangely running t4201 with your patch (without any squashing)
seems to show a breakage in shortlog.  I won't be able to come back
to this topic for at least a few hours, so this is just a single bit
"breaks" report, without "how and why" analysis, sorry.
It's because those things have tabs in their first line, so the output
now differs from the expected one exactly because of the tab-vs-space
expansion.

The wrapping logic is then also different, because the .wrapping code
does the tabs as "align to 8 chars" while the new code does tabs as
"align to 8 chars modulo the indent offset".

I only looked at the first case, but I assume the others are just more
of the same. We'd just adjust the expected output, I assume.

               Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help