search

alezost / guix.el

Emacs interface for GNU Guix package manager
https://emacs-guix.gitlab.io/website/
GNU General Public License v3.0
140 stars 16 forks source link

elisp: Add guix-deriviation. #8

Closed wigust closed 6 years ago

wigust commented 6 years ago

Demo

alezost commented 6 years ago

Hello and thank you for this feature!

Here are some notes:

I'm going to make further comments in the code.

alezost commented 6 years ago

Could you make the according changes and update your elisp-add-guix-deriviation branch? (you may just do git push --force after updating the commit)

wigust commented 6 years ago

Hello Alex,

Thank you for review!

Alex Kost notifications@github.com writes:

Hello and thank you for this feature!

At first, it should be called "derivation", not "deriviation" (note the typo you made ;-))

Done.

Please add this file to local.mk (somewhere after "guix-utils.el").

Done.

;;; guix-deriviation.el --- Guix deriviation mode

;; Copyright © 2017 Oleg Pykhalov go.wigust@gmail.com ;; Copyright © 2017 Alex Kost alezost@gmail.com

This line shouldn't be here, since I have not done anything in this file yet :-)

Done.

;;; Commentary:

;; This package provides minor-mode for prettifying Guix deriviation files.

Done.

(defcustom guix-deriviation-file-regexp-group 1

I think this variable is not needed at all. Just use 0 instead (I mean in match-beginning/match-end).

Done.

(defcustom guix-deriviation-file-regexp (rx (group (and "/gnu" "/store")

Just "/gnu/store", also the group is not needed.

Done.

(defcustom guix-deriviation-file-regexp (rx (group (and "/gnu" "/store") ;; Hash-parts do not include "e", "o", "u" and "t". See base32Chars ;; at https://github.com/NixOS/nix/blob/master/src/libutil/hash.cc "/" (= 32 (any "0-9" "a-d" "f-n" "p-s" "v-z")) (* (any "-" "." "0-9" "a-z")) ".drv"))

You missed "+" and "_" characters, so file names like these:

/gnu/store/v1lil81w3a1hp5j4hrjhmz8am7c9g7s8-ld-wrapper-x86_64-guix-linux-gnu-0.drv
/gnu/store/kvhvj09xa3rx7d15npgndcf4lg2prqpc-libstdc++-4.9.4.drv

would not be "buttonized". Overall, I would write it like this:

(rx "/gnu/store/" (= 32 (any "0-9" "a-d" "f-n" "p-s" "v-z")) (+ (any "-_+." alnum)) ".drv")

Done.

(define-button-type 'guix-deriviation-file-find 'follow-link t 'action #'guix-deriviation-file-find-button)

(defun guix-deriviation-file-find-button (button) "Guix find file BUTTON." (guix-find-file (buffer-substring (button-start button) (button-end button))))

(defun guix-make-deriviation-button ()

Since it creates multiple buttons potentially, I think using buttons in the name would be more appropriate. Also it is better to begin all symbol names with guix-derivation-, so I would call it guix-derivation-make-buttons.

Done.

(defun guix-make-deriviation-button () "Create Guix deriviation button." (guix-while-search guix-deriviation-file-regexp (make-button (match-beginning guix-deriviation-file-regexp-group) (match-end guix-deriviation-file-regexp-group) :type 'guix-deriviation-file-find)))

(defvar guix-deriviation-file-mode-map (let ((map (make-sparse-keymap))) (define-key map (kbd "") 'forward-button) (define-key map (kbd "") 'backward-button) map))

;;;###autoload (define-derived-mode guix-deriviation-file-mode special-mode "Guix-Deriviation"

I would call it just guix-derivation-mode: I think it is not very common to add -file to such modes (for example, there are scheme-mode, python-mode, but not scheme-file-mode or python-file-mode).

Done.

\{guix-deriviation-file-mode-map}" (guix-make-deriviation-button) (setq buffer-read-only nil auto-save-mode nil) (guix-pretty-print-buffer (current-buffer)) (set-buffer-modified-p nil) (setq buffer-read-only t))

Instead of setting/unsetting buffer-read-only variable, it is better to wrap the buffer-changing code like this:

  (let ((inhibit-read-only t))
    (guix-pretty-print-buffer (current-buffer)))

This is a common practice in Emacs for such things. I think setting auto-save-mode is also not needed.

Done.

alezost commented 6 years ago

(define-derived-mode guix-deriviation-file-mode special-mode "Guix-Deriviation"

I would call it just guix-derivation-mode: I think it is not very common to add -file to such modes (for example, there are scheme-mode, python-mode, but not scheme-file-mode or python-file-mode).

Done.

Oh, I didn't mean that you should rename everything to guix-derivation-mode, just the mode function itself, but not the file or other functions. Nevermind, I adjusted your commit a bit and pushed as df545b29539832a6112b0e83e4e435b48ec93797.

Thank you!

BTW, I think it would be good to add buttons for all files (not just ".drv"), WDYT?

wigust commented 6 years ago

Alex Kost notifications@github.com writes:

Oh, I didn't mean that you should rename everything to guix-derivation-mode, just the mode function itself, but not the file or other functions. Nevermind, I adjusted your commit a bit and pushed as df545b29539832a6112b0e83e4e435b48ec93797.

Oh, sorry for miss understanding.

Thank you!

BTW, I think it would be good to add buttons for all files (not just ".drv"), WDYT?

I tried this and it feels noisy for me. But we could make it configurable, I guess. So you could chose only derivations or everything else by changing predefined regexps like you did with M-x guix-edit.

Thanks, Oleg.

alezost commented 6 years ago

BTW, I think it would be good to add buttons for all files (not just ".drv"), WDYT?

I tried this and it feels noisy for me. But we could make it configurable, I guess.

I made all store file names buttonized. To reduce the noise, I removed the "underline" property from faces ("*.drv" and other files use different faces). BTW to reduce the noise even more you can use M-x guix-prettify-mode (or even global-guix-prettify-mode) :-)

If you wish to remove the highlighting of the other file names, you can either change guix-derivation-file-name face (via M-x customize-face) or set guix-derivation-file-name-faces variable accordingly.

Let me know if something is not clear or does not work properly, thanks!

wigust commented 6 years ago

Hello Alex,

Apologies for the late reply. My Gnus was broken little bit.¹

Alex Kost notifications@github.com writes:

I made all store file names buttonized. To reduce the noise, I removed the "underline" property from faces ("*.drv" and other files use different faces). BTW to reduce the noise even more you can use M-x guix-prettify-mode (or even global-guix-prettify-mode) :-)

If you wish to remove the highlighting of the other file names, you can either change guix-derivation-file-name face (via M-x customize-face) or set guix-derivation-file-name-faces variable accordingly.

It's really cool now, thank you!

Let me know if something is not clear or does not work properly, thanks!

There is also a place to improvement.

$ guix build -d hello

returns a derivation. Inside this file you could see

/gnu/store/…-hello-VERSION-guile-builder

It's a scheme file. There is no “extension” at the end of the file name. But it will be nice to prettify this file with guix-pretty-print-buffer.

What do you think we could use to determine if this file is a scheme file? There is a (begin …) expression at the beginning. Thoughts?

¹ https://lists.gnu.org/archive/html/info-gnus-english/2017-11/msg00000.html https://lists.gnu.org/archive/html/info-gnus-english/2017-11/msg00005.html

Oleg.

alezost commented 6 years ago

/gnu/store/…-hello-VERSION-guile-builder

It's a scheme file. There is no “extension” at the end of the file name. But it will be nice to prettify this file with guix-pretty-print-buffer.

What do you think we could use to determine if this file is a scheme file? There is a (begin …) expression at the beginning. Thoughts?

There are other scheme files that "want" to be indented. For example, if you open /var/guix/profiles/system/boot file (which itself is a scheme file), you find "/gnu/store/…-activate" and "/gnu/store/…-shepherd.conf" there.

So I took a straightforward approach in commit 80980e064a9d5f0fa19ad2ac033d104d42021ce8: I just added guix-scheme-mode and added it to auto-mode-alist to handle those file names.

Let me know if you find other scheme files in the store that are not handled yet, and thank you for the great idea!

wigust commented 6 years ago

There are other scheme files that "want" to be indented. For example, if you open /var/guix/profiles/system/boot file (which itself is a scheme file), you find "/gnu/store/…-activate" and "/gnu/store/…-shepherd.conf" there.

So I took a straightforward approach in commit 80980e0: I just added guix-scheme-mode and added it to auto-mode-alist to handle those file names.

Oh, it was fast a works great. Thank you!

Let me know if you find other scheme files in the store that are not handled yet, and thank you for the great idea!

Sure, thanks!