[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename
From: |
Pierre Neidhardt |
Subject: |
[bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename |
Date: |
Sun, 06 Jan 2019 20:00:19 +0100 |
User-agent: |
mu4e 1.0; emacs 26.1 |
Hi Tim,
I just reviewed your patch. Looks pretty good overall, thanks!
A few things:
> +(export package-elisp-from-package)
This should be placed at the beginning of the file in the (define-module...
See bootstrap.scm.
> +;;; Returns a package definition that packages an emacs-lisp file from the
> +;;; SRCPKG source. The package has the name PKGNAME and packages the file
> +;;; SRC-FILE from the source in its root directory as TARGET-FILE or the
> +;;; basename of SRC-FILE where INPUTS NATIVE-INPUTS and PROPAGATED-INPUTS are
> +;;; added as package inputs and SUBSTITUTIONS substitutions will be performed
> +;;; on the elisp file and SYNOPSIS and DESCRIPTION as the package synopsis
> and
> +;;; description.
> +(define* (package-elisp-from-package
Move the ";;;" comment to a docstring, e.g.
--8<---------------cut here---------------start------------->8---
(define* (package-elisp-from-package
...)
"Return ..."
--8<---------------cut here---------------end--------------->8---
> +;;; Returns a package definition that packages an emacs-lisp file from the
"Return", not "Returns".
> +;;; SRCPKG source. The package has the name PKGNAME and packages the file
Separate sentences with two spaces.
> + srcpkg pkgname src-file
Prefer complete words over abbreviations. Here I'd suggest
source-package
name
source-file
> + (synopsis (if synopsis
> + synopsis
> + (package-synopsis srcpkg)))
> + (description (if description
> + description
> + (package-description srcpkg))))))
A more Lispy way:
--8<---------------cut here---------------start------------->8---
+ (synopsis (or synopsis
+ (package-synopsis srcpkg)))
+ (description (or description
+ (package-description srcpkg))))))
--8<---------------cut here---------------end--------------->8---
Regarding the function parameters, I would turn SOURCE-FILE into SOURCE-FILES to
make it more generic. Indeed, the Emacs library could very well be split over
multiple files.
One thing I'm not too sure about is the replication of the structure fields as
keys.
I think it's easier to ignore those and let the user define them as follows:
--8<---------------cut here---------------start------------->8---
(define-public emacs-clang-rename
(package
(inherit (package-elisp-from-package
clang
"emacs-clang-rename"
"tools/clang-rename/clang-rename.el"))
(arguments ...)))
--8<---------------cut here---------------end--------------->8---
Makes sense? This would also be more robust in case the package structure
changes someday.
Finally, rebase your changes so that you directly use the last function, no
need for the clang-specific function to appear in the history of commits.
Thank you again for the good work!
--
Pierre Neidhardt
https://ambrevar.xyz/
signature.asc
Description: PGP signature
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Tim Gesthuizen, 2019/01/04
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename,
Pierre Neidhardt <=
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Tim Gesthuizen, 2019/01/06
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/07
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/07
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/07
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Ludovic Courtès, 2019/01/07
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/07
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Ludovic Courtès, 2019/01/08
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/08
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Ludovic Courtès, 2019/01/08
- [bug#33598] Optimizations for emacs-clang-format and emacs-clang-rename, Pierre Neidhardt, 2019/01/08