git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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

      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).