From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs Date: Sun, 06 Sep 2015 23:33:36 -0700 Message-ID: References: <1441479135-5285-1-git-send-email-Karthik.188@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Cc: git@vger.kernel.org, christian.couder@gmail.com, Matthieu.Moy@grenoble-inp.fr To: Karthik Nayak X-From: git-owner@vger.kernel.org Mon Sep 07 08:33:49 2015 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZYpzt-0002pz-0c for gcvg-git-2@plane.gmane.org; Mon, 07 Sep 2015 08:33:45 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753246AbbIGGdl (ORCPT ); Mon, 7 Sep 2015 02:33:41 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:34850 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751062AbbIGGdk (ORCPT ); Mon, 7 Sep 2015 02:33:40 -0400 Received: by pacfv12 with SMTP id fv12so89429897pac.2 for ; Sun, 06 Sep 2015 23:33:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-type; bh=yeB6eNJ/B7Vo0ZIffxetRx9TY6Zt4v1BZqquCahOyKQ=; b=LiLbwXExr6tQCuhPiuLkEqqCv0TQsr+OPNpHRGizVMKv5SzPioqEuX/K0mE/mAzdKH a6e+iD3U+cdrDzjZtnOBtsrWrwwoiRNRaS7UjxrLyvbhRNZlJp6rrEmF1WKoyYPoT8I9 UhWsHIcUuc8PwGR/1r2H49v6mRgSDl7XhUc0LkkB0Iy6g4dGYWy0ojuNfAr1aFQBb2rM keWNBpLXqPIucA5HONaloZoz2h1zNDgoTqWncGFQn6SXQjnf1frIsGlXbQwVLPA6STRD 8k5yCTxNVZJGjjJnWAePQ96dGF6Kd5LsVhZ6BaqmYgKnimqORDFm1kM+POLQl5r5cOiS wm1g== X-Received: by 10.66.253.170 with SMTP id ab10mr42414046pad.135.1441607619925; Sun, 06 Sep 2015 23:33:39 -0700 (PDT) Received: from localhost ([2620:0:1000:861b:751a:a433:a60b:dba1]) by smtp.gmail.com with ESMTPSA id q3sm10614668pdg.89.2015.09.06.23.33.37 (version=TLS1_2 cipher=AES128-SHA256 bits=128/128); Sun, 06 Sep 2015 23:33:37 -0700 (PDT) In-Reply-To: <1441479135-5285-1-git-send-email-Karthik.188@gmail.com> (Karthik Nayak's message of "Sun, 6 Sep 2015 00:22:01 +0530") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Karthik Nayak writes: > diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt > index d039f40..c5154bb 100644 > --- a/Documentation/git-for-each-ref.txt > +++ b/Documentation/git-for-each-ref.txt > @@ -128,13 +128,14 @@ color:: > are described in `color.branch.*`. > > align:: > - Left-, middle-, or right-align the content between %(align:..) > + Left-, middle-, or right-align the content between %(align:...) > and %(end). Followed by `:,`, where the > `` is either left, right or middle and `` is > the total length of the content with alignment. If the > contents length is more than the width then no alignment is > performed. If used with '--quote' everything in between > - %(align:..) and %(end) is quoted. > + %(align:...) and %(end) is quoted, but if nested then only the > + topmost level performs quoting. I am not sure if the description of quote belongs to "align". Isn't the general rule at the endgame when there are more opening things that would buffer its effect up to the corresponding %(end): - Some atoms like %(align) and %(if) always require a matching %(end). We call them "opening atoms" and sometimes denote them as %($open). - When a scripting language specific quoting is in effect, except for opening atoms, replacement from every %(atom) is quoted when and only when it appears at the top-level (that is, when it appears outside %($open)...%(end)). - When a scripting language specific quoting is in effect, everything between a top-level opening atom and its matching %(end) is evaluated according to the semantics of the opening atom and its result is quoted. To put the third item above in a different way, a matching %($open)... %(end) pair behaves as if it is a single normal atom, from the point of view of the quoting rule. All top-level atoms are invidivually quoted, whether they are normal atoms or open/end pairs. And that rule is not limited to %(align). I am flexible with the terminology, but the point is that I think the quoting rules are better be specified _outside_ the description of a particular atom, but as a general rule. > diff --git a/builtin/tag.c b/builtin/tag.c > index 9fa1400..f55dfda 100644 > --- a/builtin/tag.c > +++ b/builtin/tag.c > @@ -43,8 +43,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con > > if (!format) { > if (filter->lines) > - format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) %%(contents:lines=%d)", > - filter->lines); > + format = to_free = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) " > + "%%(contents:lines=%d)", filter->lines); This line still looks overlong. Would it help to stop spelling this as a double "a = b = overlong expression" assignment? > /* > * Perform quote formatting when the stack element is that of > - * a modifier atom and right above the first stack element. > + * a supporting atom. If nested then perform quote formatting > + * only on the topmost supporting atom. > */ > if (!state->stack->prev->prev) { > quote_formatting(&s, current->output.buf, state->quote_style); > @@ -261,6 +262,22 @@ static void end_atom_handler(struct atom_value *atomv, struct ref_formatting_sta > pop_stack_element(&state->stack); > } > > +static int match_atom_name(const char *name, const char *atom_name, const char **val) > +{ > + const char *body; > + > + if (!skip_prefix(name, atom_name, &body)) > + return 0; /* doesn't even begin with "atom_name" */ > + if (!body[0] || !body[1]) { > + *val = NULL; /* %(atom_name) and no customization */ > + return 1; > + } This if condition is puzzling. !body[0] would mean the name was exactly atom_name, which is what you want to catch. But the other part does not make any sense to me. what does (body[0] != '\0' && !body[1]) mean? atom_name asked for was "align" and name was "aligna"? That is not "%(align) and no customization". > + if (body[0] != ':') > + return 0; /* "atom_namefoo" is not "atom_name" or "atom_name:..." */ This one makes sense to me. > + *val = body + 1; /* "atomname:val" */ Spell that "atom_name:val" perhaps?