[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [v2 1/1] gnu: Add peg-markdown.
From: |
Ludovic Courtès |
Subject: |
Re: [v2 1/1] gnu: Add peg-markdown. |
Date: |
Sun, 29 Nov 2015 11:21:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Leo Famulari <address@hidden> skribis:
> * gnu/packages/markdown.scm (peg-markdown): New variable.
[...]
> + ;; The Makefile offers a memory leak test using
> + ;; Valgrind.
> + (add-after 'check 'leak-check
> + (lambda _
> + (zero? (system* "make" "leak-check"))))
The opening parenthesis can be aligned below the ‘d’ of ‘add-after’.
> + (replace
> + 'install
> + (lambda* (#:key outputs #:allow-other-keys)
Ditto, with ‘install’ moved to the previous line.
> + ;; The Makefile does not check if the output paths
> exist.
> + (let* ((out (assoc-ref %outputs "out"))
> + (bin (string-append out "/bin"))
> + (doc (string-append out "/share/man/man1")))
> + (mkdir-p bin)
> + ;; The top-level Makefile does not have an install
> + ;; target, so we have to do this manually.
> + (zero?
> + (system* "make" "-C" "peg-0.1.4"
> + (string-append "PREFIX=" %output)
> + "CC=gcc" "install"))
> + ;; The Makefile does not install the manpage.
> + (install-file "peg-0.1.4/peg.1" doc)
> + ;; The manpage applies to both peg and leg.
> + (symlink
> + (string-append doc "/peg.1")
> + (string-append doc "/leg.1"))
> + #t))))))
The return value of ‘zero?’ is ignored here.
To address this and clarify the code, what about making the
symlink-manpage thing a separate phase, added after ‘install’?
> + (synopsis "Implementation of markdown in C, using a PEG grammar")
Capitalize “Markdown” (?); I think it’s fine to remove “using a PEG
grammar”, which is more of an implementation detail and is explained in
the description.
> + (description "This is an implementation of John Gruber's markdown in C.
> It
> +uses a parsing expression grammar (PEG) to define the syntax. This should
> allow
Make sure lines are below 80 chars.
Could you send an updated patch?
Thanks,
Ludo’.