git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Karthik Nayak <karthik.188@gmail.com>,
	git@vger.kernel.org, christian.couder@gmail.com
Subject: Re: [PATCH v5 03/11] ref-filter: add option to pad atoms to the right
Date: Mon, 27 Jul 2015 08:45:42 -0700	[thread overview]
Message-ID: <xmqqr3ntioyh.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <vpq3809aho9.fsf@anie.imag.fr> (Matthieu Moy's message of "Mon, 27 Jul 2015 14:50:14 +0200")

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> See my remark on previous patch: this test is not sufficient. You do
> not only need to check that %(padright) is taken into account, but you
> also need to check that it is taken into account for the right atom and
> only this one. I'd suggest
>
> --format '%(refname)%(padright:25)|%(refname)|%(refname)|'
>
> Only the middle column should be padded.

This actually raises an interesting point.  It is equally valid to
view that format string as requesting to pad the first "|" with 24
spaces; in other words, %(padright:N) would apply to the next span,
be it a literal string up to the next %(atom), or the %(atom) that
comes immediately after it.

The thing is, the above _might_ be an OK way to ask the middle
refname to be padded, but I somehow find it very unnatural and
unintuitive to expect that this:

	--format '%(padright:25)A Very Long Irrelevancy%(refname)'

would do nothing to "A Very Long Irrelevancy" and affects the
refname that comes much later in the format string.

Or we could simply forbid certain atoms followed by an non-empty
literal as an error.

My preference between the three is "%(padright:4)", etc. to apply to
the "next span", but I can live with "it is an error to pad-right
a far-away %(atom)".

  reply	other threads:[~2015-07-27 15:45 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  7:26 [PATCH v5 00/11] port tag.c to use ref-filter APIs Karthik Nayak
2015-07-27  7:27 ` [PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state' Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 02/11] ref-filter: make `color` use `ref_formatting_state` Karthik Nayak
2015-07-27 12:47     ` Matthieu Moy
2015-07-27 14:28       ` Junio C Hamano
2015-07-27 17:23         ` Karthik Nayak
2015-07-27 16:02       ` Karthik Nayak
2015-07-27 13:06     ` Matthieu Moy
2015-07-27 17:16       ` Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 03/11] ref-filter: add option to pad atoms to the right Karthik Nayak
2015-07-27 12:50     ` Matthieu Moy
2015-07-27 15:45       ` Junio C Hamano [this message]
2015-07-27 15:54         ` Matthieu Moy
2015-07-27 18:42           ` Karthik Nayak
2015-07-27 18:47             ` Matthieu Moy
2015-07-27 18:54               ` Karthik Nayak
2015-07-27 18:56           ` Junio C Hamano
2015-07-27 18:43       ` Karthik Nayak
2015-07-28 21:41     ` Eric Sunshine
2015-07-29 16:16       ` Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 04/11] ref-filter: add option to filter only tags Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 05/11] ref-filter: support printing N lines from tag annotation Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 06/11] ref-filter: add support to sort by version Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 07/11] ref-filter: add option to match literal pattern Karthik Nayak
2015-07-27 12:54     ` Matthieu Moy
2015-07-27 15:57       ` Karthik Nayak
2015-07-27 15:59         ` Karthik Nayak
2015-07-27 16:06           ` Matthieu Moy
2015-07-27 16:48             ` Karthik Nayak
2015-07-28 21:49     ` Eric Sunshine
2015-07-29 16:17       ` Karthik Nayak
2015-07-30 11:21       ` Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 08/11] tag.c: use 'ref-filter' data structures Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 09/11] tag.c: use 'ref-filter' APIs Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 10/11] tag.c: implement '--format' option Karthik Nayak
2015-07-27  7:27   ` [PATCH v5 11/11] tag.c: implement '--merged' and '--no-merged' options Karthik Nayak
2015-07-27 12:42   ` [PATCH v5 01/11] ref-filter: introduce 'ref_formatting_state' Matthieu Moy
2015-07-27 15:28     ` Karthik Nayak

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=xmqqr3ntioyh.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.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).