git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH v4 0/5] Disable "git status" comment prefix
Date: Thu, 5 Sep 2013 19:23:22 -0400	[thread overview]
Message-ID: <20130905232322.GB29351@sigill.intra.peff.net> (raw)
In-Reply-To: <vpqioyfukkw.fsf@anie.imag.fr>

On Thu, Sep 05, 2013 at 09:36:47PM +0200, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > One caveat, though.  The name "oldStyle" will become problematic,
> > when we want to remove some wart in the output format long after
> > this "no comment prefix by default" series lands.  Some people may
> > expect setting oldStyle=true would give output from 1.8.4 era, while
> > some others would expect that it would give output from 1.8.5 era
> > that does not have comment prefix but still has that wart we will
> > remove down the line.
> 
> I'm fine with any name actually (since it is enabled by default, people
> don't need to know the name to benefit from the new output). Maybe
> status.displayCommentPrefix was the best name after all.

FWIW, I had the same thought as Junio. I much prefer something like
status.displayCommentPrefix for clarity and future-proofing.

As for the feature itself, I am still undecided whether I like it. I've
tried looking at the output of the series, and it feels weird to me.

Part of it is undoubtedly that my brain is simply used to the other way.
But it also seems to drop some of the vertical whitespace, which makes
things feel very crowded. E.g., before:

  # On branch private
  # Your branch and 'origin/next' have diverged,
  # and have 472 and 59 different commits each, respectively.
  #
  # Untracked files:
  #       t/foo
  #       test-obj-pool
  #       test-string-pool
  #       test-treap
  #       test-url-normalize
  nothing added to commit but untracked files present

after:

  On branch private
  Your branch and 'origin/next' have diverged,
  and have 472 and 59 different commits each, respectively.
  Untracked files:
          t/foo
          test-obj-pool
          test-string-pool
          test-treap
          test-url-normalize
  nothing added to commit but untracked files present

The blank before "Untracked files" was dropped (was this intentional? I
didn't see any note of it in the discussion), and the bottom "nothing
added" line butts against the untracked list more obviously, because
they now all have the same comment indentation.

I wonder if it would look a little nicer as:

  On branch private
  Your branch and 'origin/next' have diverged,
  and have 472 and 59 different commits each, respectively.

  Untracked files:
          t/foo
          test-obj-pool
          test-string-pool
          test-treap
          test-url-normalize

  nothing added to commit but untracked files present

As an orthogonal thing to your patch, I feel like the first two items
(branch and ahead/behind) are kind of squished and oddly formatted (in
both the original and yours). Could we do something like:

  Your branch 'private' and its upstream 'origin/next' have diverged,
  and have 472 and 59 different commits each, respectively.

when we are going to print both?  That's 69 characters, which might
overrun 80 if you have long branch names, but we could also line-break
it differently.

That doesn't need to be part of your topic, but while we are talking
about the format of the message, maybe it is worth thinking about.

-Peff

  parent reply	other threads:[~2013-09-05 23:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-05  9:50 [PATCH v4 0/5] Disable "git status" comment prefix Matthieu Moy
2013-09-05  9:50 ` [PATCH v4 1/5] builtin/stripspace.c: fix broken indentation Matthieu Moy
2013-09-05  9:50 ` [PATCH v4 2/5] wt-status: use argv_array API Matthieu Moy
2013-09-05  9:50 ` [PATCH v4 3/5] submodule summary: ignore --for-status option Matthieu Moy
2013-09-05  9:50 ` [PATCH v4 4/5] status: disable display of '#' comment prefix by default Matthieu Moy
2013-09-05  9:50 ` [PATCH v4 5/5] tests: don't set status.oldStyle file-wide Matthieu Moy
2013-09-05 12:35 ` [PATCH v4 0/5] Disable "git status" comment prefix Matthieu Moy
2013-09-05 19:13 ` Junio C Hamano
2013-09-05 19:36   ` Matthieu Moy
2013-09-05 23:09     ` Junio C Hamano
2013-09-05 23:23     ` Jeff King [this message]
2013-09-06 16:44       ` Matthieu Moy
2013-09-06 16:53       ` Jonathan Nieder
2013-09-06 17:28         ` Matthieu Moy
2013-09-06 18:12           ` Junio C Hamano
2013-09-06 18:43             ` Matthieu Moy
2013-09-06 21:55           ` Jeff King

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130905232322.GB29351@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --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.
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 NNTP newsgroup(s).