From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 3/2] vreportf: add prefix to each line
Date: Thu, 12 Jan 2017 01:59:47 -0500 [thread overview]
Message-ID: <20170112065947.qgukcozdi333yqgi@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqpojtmeld.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 11, 2017 at 02:11:42PM -0800, Junio C Hamano wrote:
> > It's actually kind of ugly. For instance, a failing test in
> > t3600 now produces:
> >
> > error: the following files have staged content different from both the
> > error: file and the HEAD:
> > error: bar.txt
> > error: foo.txt
> > error: (use -f to force removal)
> >
> > which seems a bit aggressive.
>
> I agree that it is ugly, but one reason I was hoping to do this
> myself (or have somebody else do it by procrastinating) was that I
> thought it may help i18n. That is, for an original
>
> error(_("we found an error"))
>
> a .po file may translate the string to a multi-line string that has
> LFs in it and the output would look correct. The translator already
> can do so by indenting the second and subsequent lines by the same
> column-width as "error: " (or its translation in their language, if
> we are going to i18n these headers), but that (1) is extra work for
> them, and (2) makes it impossible to have the same message appear in
> different contexts (i.e. "error:" vs "warning:" that have different
> column-widths).
Yes, I agree that would be a functional benefit. I'm just hoping we can
do it in a way that is visually pleasing.
> > It also screws up messages which indent with tabs (the prefix eats
> > up some of the tabstop, making the indentation smaller).
>
> This is unavoidable and at the same time is a non-issue, isn't it?
> Messages that indent the second and subsequent lines with tabs are
> compensating the lack of the multi-line support of vreportf(), which
> this RFC patch fixes. They may need to be adjusted to the new world
> order, but that is a good thing. New multi-line messages no longer
> have to worry about the prefix that is added only to the first line
> when continuing the message to multiple lines.
I'm not so sure it is just about compensating. Look at the message
quoted above. The original looks like:
error: the following files have staged content different from both the
file and the HEAD:
bar.txt
foo.txt
(use -f to force removal)
The leading whitespace is visually separating the list of files, not
just from the line with "error:", but from the other lines.
Though I think if we replaced tabs with spaces in this instance, then
they would still be bumped relative to the rest of the text.
> > It may be possible to address some of that by using some
> > other kind of continuation marker (instead of just repeating
> > the prefix), and expanding initial tabs.
>
> Yes indeed. The "some other kind of continuation marker" could just
> be a run of spaces that fill the same column as the "error: " or
> other prefix given to the first line.
I tried that, along with several other variants, but it gets
confusing/ugly when mixed with indentation that is significant to the
line. For example, the t3600 message becomes:
error: the following files have staged content different from both the
file and the HEAD:
bar.txt
foo.txt
(use -f to force removal)
Which is arguably better than what we have now, but still looks pretty
bad to me.
I wonder if it would help for the marker to end in a non-whitespace
character. Like:
error: the following files have staged content different from both the
: file and the HEAD:
: bar.txt
: foo.txt
: (use -f to force removal)
or something. The ":" is a bit sparse looking. Maybe there are better
options. That does ruin line-by-line selection for cut-and-paste,
though.
So I dunno. I am open to ideas, but I didn't find one that I really
liked (this patch is actually a leftover from before Christmas, so I
don't even remember all the things I tried, just that I didn't like any
of them ;) ).
-Peff
prev parent reply other threads:[~2017-01-12 6:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-11 14:01 [PATCH 0/2] sanitizing error message contents Jeff King
2017-01-11 14:02 ` [PATCH 1/2] Revert "vreportf: avoid intermediate buffer" Jeff King
2017-01-11 14:02 ` [PATCH 2/2] vreport: sanitize ASCII control chars Jeff King
2017-01-11 14:07 ` [RFC PATCH 3/2] vreportf: add prefix to each line Jeff King
2017-01-11 22:11 ` Junio C Hamano
2017-01-12 6:59 ` Jeff King [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170112065947.qgukcozdi333yqgi@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).