Date: Mon, 28 Jan 2019 20:48:20 +0000
To: =?UTF-8?Q?Cristian_Adri=C3=A1n_Ontivero?= <cristianontivero@gmail.com>
From: Simon Ser <contact@emersion.fr>
Cc: "~emersion/mrsh-dev@lists.sr.ht" <~emersion/mrsh-dev@lists.sr.ht>
Reply-To: Simon Ser <contact@emersion.fr>
Subject: Re: [PATCH v2] Parse arithmetic expressions with shifts
Message-ID: <U7ts9Yh_Z3XOiaWl6AhxSN67-FxIMtbaYeTlvyyyw5DQAS4ikfnS4TYaWhLzU3L__Qchoi8zRWD-ikFm8bmHtmYtoDUqVmfVKpSKHCpQSn0=@emersion.fr>
In-Reply-To: <20190125085755.5290-1-cristianontivero@gmail.com>
References: <20190125085755.5290-1-cristianontivero@gmail.com>
Feedback-ID: FsVprHBOgyvh0T8bxcZ0CmvJCosWkwVUg658e_lOUQMnA9qynD8O1lGeniuBDfPSkDAUuhiKfOIXUZBfarMyvA==:Ext:ProtonMail
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Friday, January 25, 2019 9:57 AM, Cristian Adri=C3=A1n Ontivero <cristia=
nontivero@gmail.com> wrote:
> We introduce the function arithmetic_word() to parse arithmetic
> expressions instead of reusing the general word(), and generalize
> word_list() to receive a pointer to function, so that word_list() may be
> used to parse a list of whatever type of word we need.
>
> This fixes #51, and enables properly parsing parenthesized expressions
> inside arithmetic expressions, e.g. $(((2+1)-1)).
> ---
>
> This is mostly a working proof of concept. There is (as might be expected=
) quite
> a bit of similarity between word() and arithmetic_word(), but I think tha=
t it
> would be better in the long term to remove the "end" parameter, and have =
a
> couple of different *_word() functions that are called whenever appropria=
te.
>
> As discussed, the alternative would be adding a mrsh_word_type parameter =
to
> word() and word_list(), but I think that this would eventually lead to a =
word()
> function with a lot of if-statements for each distinct context, and might=
 lead
> to a more complex word() function (although preventing the repetition int=
rinsic
> to the *_word() functions alternative).
>
> What do you think?

+1, I like it. Yeah, I agree, the `end` parameter should go away.

Is there anything that prevents this from being merged?

>
>  include/parser.h |   3 ++
>  parser/word.c    | 120 +++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 118 insertions(+), 5 deletions(-)
>
> diff --git a/include/parser.h b/include/parser.h
> index 56e549f..d4b32b9 100644
> --- a/include/parser.h
> +++ b/include/parser.h
> @@ -63,6 +63,8 @@ struct mrsh_parser {
>  =09void *alias_user_data;
>  };
>
> +typedef struct mrsh_word * (*word_fn)(struct mrsh_parser *, char end);

Nitpick: the convention I've used so far is to use the `_func` prefix
for function types.