Message ID | 15d01b32313f5f2f291b120597719ae92bd26acd.1630639896.git.iskarian@mgsn.dev |
---|---|
State | New |
Headers | show |
Series | [bug#50349] packages: Add 'define-package' syntax. | expand |
Context | Check | Description |
---|---|---|
cbaines/applying patch | fail | View Laminar job |
cbaines/issue | success | View issue |
Hi Sarah, On Thu, 02 Sep 2021 at 21:06, Sarah Morgensen <iskarian@mgsn.dev> wrote: > (define-package my-favorite-package > (name "my-favorite-package") > ...) > > The purpose is primarily to save the horizontal indent, but IMO it looks > better, and is marginally more clear for newcomers. I think ideally we could > eventually transition to using this syntax as the primary syntax and only use > 'define-public' when necessary. On one hand, I think it is a good idea; especially for newcomers. On the other hand, it will break ’git-blame’, isn’t it? Therefore, I am not convinced such change is worth for ’gnu/packages/’. Instead it seems worth only for teaching custom packages. Explaining to people in my labs, they are often confused between ’define’ and ’define-public’. But then, there is two “styles” and people could be more confused. Well, my feelings are mixed. Thanks for opening the discussion. :-) Cheers, simon
Hi, zimoun <zimon.toutoune@gmail.com> writes: > Hi Sarah, > > On Thu, 02 Sep 2021 at 21:06, Sarah Morgensen <iskarian@mgsn.dev> wrote: > >> (define-package my-favorite-package >> (name "my-favorite-package") >> ...) >> >> The purpose is primarily to save the horizontal indent, but IMO it looks >> better, and is marginally more clear for newcomers. I think ideally we could >> eventually transition to using this syntax as the primary syntax and only use >> 'define-public' when necessary. > > On one hand, I think it is a good idea; especially for newcomers. On > the other hand, it will break ’git-blame’, isn’t it? Yes, there would be a one-time discontinuity. Reformats like this can be ignored with the `--ignore-ref' option, or with a file and a config option: .git-blame-ignore-revs: --8<---------------cut here---------------start------------->8--- # Convert 'define-public' forms to 'define-package' forms 15d01b32313f5f2f291b120597719ae92bd26acd --8<---------------cut here---------------end--------------->8--- .git/config: --8<---------------cut here---------------start------------->8--- [blame] ignoreRevsFile = .git-blame-ignore-revs --8<---------------cut here---------------end--------------->8--- We could include the latter in e.g. a `.gitconfig' file committed to the repo, but in order to use config settings from the file, users would have to first run git config --local include.path ../.gitconfig Thankfully, this only has to be done once per clone. If you really wanted to make sure it got run, I suppose you could add it to a 'make' target. -- Sarah
Sarah Morgensen schreef op do 02-09-2021 om 21:06 [-0700]: > Hello Guix, > > This patch adds a shorthand for "(define-public name (package ...))": > > (define-package my-favorite-package > (name "my-favorite-package") > ...) This could be even shorter in the special case that the variable name and package name are the same (modulo types): (define-package "my-favorite-package" (version ...) ...) 'datum->syntax' and 'string->symbol' can be used to turn "my-favorite-package" into an identifier. A 'define-unexported-package' might be required in some places. > The purpose is primarily to save the horizontal indent, but IMO it looks > better, and is marginally more clear for newcomers. I think ideally we could > eventually transition to using this syntax as the primary syntax and only use > 'define-public' when necessary. > > There are some downsides... it's one more form to keep track of, and 'let' > forms can't easily be used with it. > > Since it's a syntax rule, it doesn't cause packages to rebuild (tested). I've > also tested the indentation rules, indent-code.el, and the snippets. > > This probably deserves a documentation addition, but I wasn't sure where to > add it without confusing newcomers. Suggestions welcome! ‘Defining Packages’ would be a good place I think. > What do you all think? This looks nice to me. IIUC, the define-package is intended to be clearer to newcomers, so you might want to ask for feedback on the new syntax on help-guix@gnu.org. Greetings, Maxime.
diff --git a/.dir-locals.el b/.dir-locals.el index aaa48ab552..8141cf4fc2 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -71,6 +71,7 @@ (eval . (put 'with-writable-file 'scheme-indent-function 2)) (eval . (put 'package 'scheme-indent-function 0)) + (eval . (put 'define-package 'scheme-indent-function 1)) (eval . (put 'package/inherit 'scheme-indent-function 1)) (eval . (put 'origin 'scheme-indent-function 0)) (eval . (put 'build-system 'scheme-indent-function 0)) diff --git a/etc/indent-code.el b/etc/indent-code.el index bdea8ee8bf..b83659f2f9 100755 --- a/etc/indent-code.el +++ b/etc/indent-code.el @@ -94,7 +94,7 @@ ;; Indent the definition of PACKAGE-NAME in FILE-NAME. (find-file file-name) (goto-char (point-min)) - (if (re-search-forward (concat "^(define\\(\\|-public\\) +" + (if (re-search-forward (concat "^(define\\(\\|-public\\|-package\\) +" package-name) nil t) (let ((indent-tabs-mode nil)) diff --git a/etc/snippets/text-mode/guix-commit-message-add-cl-package b/etc/snippets/text-mode/guix-commit-message-add-cl-package index e255736b05..eb0de677e7 100644 --- a/etc/snippets/text-mode/guix-commit-message-add-cl-package +++ b/etc/snippets/text-mode/guix-commit-message-add-cl-package @@ -7,7 +7,7 @@ gnu: Add ${1:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") (beginning-of-buffer) - (when (search-forward "+(define-public " nil 'noerror) + (when (re-search-forward "+(define-\\(public\\|package\\) " nil 'noerror) (replace-regexp-in-string "^sbcl-" "" (thing-at-point 'sexp 'no-properties))))`}. diff --git a/etc/snippets/text-mode/guix-commit-message-add-package b/etc/snippets/text-mode/guix-commit-message-add-package index 7cebd4023a..11aeceb129 100644 --- a/etc/snippets/text-mode/guix-commit-message-add-package +++ b/etc/snippets/text-mode/guix-commit-message-add-package @@ -7,7 +7,7 @@ gnu: Add ${1:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") (goto-char (point-min)) - (when (re-search-forward "\\+(define-public \\(\\S-+\\)" nil 'noerror) + (when (re-search-forward "\\+(define-\\(?:public\\|package\\) \\(\\S-+\\)" nil 'noerror) (match-string-no-properties 1)))`}. * `(car (magit-staged-files))` ($1): New variable. \ No newline at end of file diff --git a/etc/snippets/text-mode/guix-commit-message-rename-package b/etc/snippets/text-mode/guix-commit-message-rename-package index 9695ca1b3d..2315443573 100644 --- a/etc/snippets/text-mode/guix-commit-message-rename-package +++ b/etc/snippets/text-mode/guix-commit-message-rename-package @@ -7,12 +7,12 @@ gnu: ${1:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") (beginning-of-buffer) - (when (search-forward "-(define-public " nil 'noerror) + (when (re-search-forward "-(define-\\(public\\|package\\) " nil 'noerror) (thing-at-point 'sexp 'no-properties)))`}: Rename package to ${2:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") (beginning-of-buffer) - (when (search-forward "+(define-public " nil 'noerror) + (when (re-search-forward "+(define-\\(public\\|package\\) " nil 'noerror) (thing-at-point 'sexp 'no-properties)))`}. * `(car (magit-staged-files))` ($1): Define in terms of diff --git a/etc/snippets/text-mode/guix-commit-message-update-package b/etc/snippets/text-mode/guix-commit-message-update-package index f187419aa2..1d39e28b77 100644 --- a/etc/snippets/text-mode/guix-commit-message-update-package +++ b/etc/snippets/text-mode/guix-commit-message-update-package @@ -8,7 +8,7 @@ gnu: ${1:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") (goto-char (point-min)) - (when (re-search-forward "(define-public \\(\\S-+\\)" nil 'noerror) + (when (re-search-forward "(define-\\(?:public\\|package\\) \\(\\S-+\\)" nil 'noerror) (match-string-no-properties 1)))`}: Update to ${2:`(with-temp-buffer (magit-git-wash #'magit-diff-wash-diffs "diff" "--staged") diff --git a/guix/packages.scm b/guix/packages.scm index c825f427d8..ecd0b7e47d 100644 --- a/guix/packages.scm +++ b/guix/packages.scm @@ -6,6 +6,7 @@ ;;; Copyright © 2017, 2019, 2020 Efraim Flashner <efraim@flashner.co.il> ;;; Copyright © 2019 Marius Bakke <mbakke@fastmail.com> ;;; Copyright © 2021 Chris Marusich <cmmarusich@gmail.com> +;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev> ;;; ;;; This file is part of GNU Guix. ;;; @@ -99,6 +100,7 @@ (define-module (guix packages) package-supported-systems package-properties package-location + define-package hidden-package hidden-package? package-superseded @@ -425,6 +427,12 @@ (define-record-type* <package> package) 16))))) +(define-syntax-rule (define-package name body ...) + "Equivalent to (define-public name (package body ...))." + (define-public name + (package + body ...))) + (define-syntax-rule (package/inherit p overrides ...) "Like (package (inherit P) OVERRIDES ...), except that the same transformation is done to the package P's replacement, if any. P must be a bare