Subject: Re: [bug#44599] [PATCH] gnu: Add emacs-parsec.

From: Nicolas Goaziou <[email-masked]>
Date: Sat, 14 Nov 2020 21:14:34 +0100

Hello,

Adam Kandur via Guix-patches via <[email-masked]> writes:

> Subject: [PATCH] gnu: Add emacs-parsec.

Thank you. Some comments follow.

> +(define-public emacs-parsec
> +  (let ((revision "0")
> +        (commit "2cbbbc2254aa7bcaa4fb5e07c8c1bf2f381dba26"))
> +    ;; no taged branch

Usually, the comment is above the `let', because you expect it to
comment what is located below.

Also, this comment is not true. There are tags, the last one being
"0.1.3". If you think we need the latest commit, you can write that the
last release is too old instead.

> +    (package
> +      (name "emacs-parsec")
> +      (build-system emacs-build-system)

This is an unusual place for `build-system'. You may want to move it
below the `source' part.

> +      (version (git-version "0.1.3" revision commit))
> +      (source
> +       (origin
> +	 (method git-fetch)
> +	 (uri (git-reference
> +	       (url "https://github.com/cute-jumper/parsec.el")
> +	       (commit commit)))
> +	 (file-name (git-file-name name version))
> +	 (sha256
> +          (base32 "1g1s8s45g3kkbi3h7w0pmadmzdswb64mkdvdpg2lihg341kx37gm"))))

The indentation looks a bit off.

> +      (home-page "https://github.com/cute-jumper/parsec.el")
> +      (synopsis "A parser combinator library for Emacs Lisp, similar to Haskell's Parsec library.")

Synopsis cannot start with "A", and should not end with a full stop. You
may want to run "guix lint" on this.

The synopsis could be: "Parser combinator library for Emacs Lisp"

> +      (description "This library, however, contains most of the parser combinators in Text.Parsec.Combinator, 
> +which should be enough in most use cases. Of course more combinators can be added if necessary! 
> +Most of the parser combinators have the same behavior as their Haskell counterparts. 
> +@code{parsec} also comes with a simple error handling mechanism so that it can display an error message showing how the parser fails.")

The description seems to depend on the synopsis, but it should be
stand-alone. What about:

     Parsec is a parser combinator library for Emacs Lisp, similar to
     Haskell's Parsec library.  It contains most of the parser
     combinators in Text.Parsec.Combinator, and more combinators can be
     added if necessary!  Most of the parser combinators have the same
     behavior as their Haskell counterparts.  Parsec also comes with
     a simple error handling mechanism so that it can display an error
     message showing how the parser fails.

Mind the two spaces after sentences!

> +      (license license:gpl3))))

License is actually gpl3+.

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou