From: Junio C Hamano <gitster@pobox.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
Git List <git@vger.kernel.org>,
Christian Couder <christian.couder@gmail.com>,
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCH v11 05/13] ref-filter: implement an `align` atom
Date: Mon, 17 Aug 2015 11:39:35 -0700 [thread overview]
Message-ID: <xmqqa8tpep3s.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAOLa=ZRfA-8_w6VKgWQsoL7TrdyjEq5LTHwas=_04tmx9MWhqA@mail.gmail.com> (Karthik Nayak's message of "Mon, 17 Aug 2015 19:58:10 +0530")
Karthik Nayak <karthik.188@gmail.com> writes:
> On Mon, Aug 17, 2015 at 7:37 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> ...
>> Second, I realize that Junio suggested the 'return_to' idea, but it
>> seems like it could become overly painful since each handler of this
>> sort is going to have to perform the same manipulation to append its
>> collected output to its parent state's output. What if you instead
>> make it the responsibility of pop_state() to append the 'output' from
>> the state being popped to the "prev" state's 'output'? This way, it
>> happens automatically, thus reducing code in each individual handler,
>> and reducing the burden of having to keep writing the same code.
>
> Good question, what if we don't want to append to strbuf at all?
> For e.g., We were discussing an "%(if).....%(then)......%(end)"
> atom structure, here if the if condition isn't met we wouldn't want to
> append to the prev strbuf, hence I thought it's better if the handler
> decided whether or not to append to prev strbuf.
I'd imagine the implementation of these to be along the lines of
(thinking aloud):
- "%(if:[nonempty|empty|...])" pushes a new stack, and sets its
attend/at_end/end_scope function to signal a syntax error. It
also records what condition (e.g. "nonempty") to use in the new
stack.
- "%(then)" inspects the top-of-stack output and uses the condition
recorded by the %(if) that created the stack to decide true or
false. The stack element pushed by %(if) is then removed.
Notice that the end_scope function prepared by %(if) is never
called.
Then (no pun intended):
- If true, that means we would want the (expansion of) text up to
"%(end)" or "%(else)", whichever comes first, appended to the
surrounding output. Push a new stack and set its end_scope
function to the one that appends the top-of-stack output to the
surrounding output, expecting %(end) will follow without
%(else).
- If false, that means we would want the (expansion of) text up
to "%(end)" or "%(else)", whichever comes first, discarded.
Push a new stack and set its end_scope function to the one that
discards the top-of-stack output, expecting %(end) will follow
without %(else).
- "%(else)" inspects the top of the stack, and if it is not left by
"%(then)", signal a syntax error.
Else (no pun intended), it runs the end_scope function left by
"%(then)" on the top-of-stack output (e.g. if "%(then)" found
that the condition holds true, the accumulated output at this
point should be appended to the surrounding output, and it was
expected to be done by "%(end)" if this "%(else)" weren't
present. We do it here before waiting for "%(end)").
Truncate the top-of-stack output, flip the end_scope function to
the one opposite from the one left by "%(then)". And let "%(end)"
take care of it.
- "%(end)" just unconditionally runs the end_scope function on the
top of the stack output.
Eric's suggestion is let the caller of the end_scope function to
always append the output of the top-of-stack, and I think it makes
sense. It makes a common "%(atom)" implementation simpler. Things
like %(then) and %(else) above need to make sure that they reset the
top-of-stack output to empty as necessary, but that is not such a
huge implementation burden---their operation is already unusual and
needs to be more complex than the plain-vanilla %(atom)s anyway.
next prev parent reply other threads:[~2015-08-17 18:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-15 18:00 [PATCH v11 05/13] ref-filter: implement an `align` atom Karthik Nayak
2015-08-16 11:46 ` Karthik Nayak
2015-08-16 12:04 ` Andreas Schwab
2015-08-16 23:49 ` Eric Sunshine
2015-08-17 2:07 ` Eric Sunshine
2015-08-17 14:28 ` Karthik Nayak
2015-08-17 18:22 ` Eric Sunshine
2015-08-17 18:39 ` Junio C Hamano [this message]
2015-08-17 19:12 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa8tpep3s.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 \
--cc=sunshine@sunshineco.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.