Closed wigust closed 6 years ago
Hello, Oleg!
I have not looked at the code closely yet, but I have several comments. At first, thank you! I often wanted to have a possibility to "indent" all these guix paths that I saw here and there.
I see that you don't support more complicated paths that are met in
profile files (~/.guix-profile/etc/profile
), I mean the paths that
begin with ${GUIX_PROFILE
. Or will the regexp be too complex (if
possible) for these file names?
Also I noticed that when a line is commented, guix-env-var-mode
does not handle it properly:
before:
# export PATH="/gnu/store/6r16pi4zsxdf4a9p4v7padp2nimxa5jl-profile/bin:/gnu/store/6r16pi4zsxdf4a9p4v7padp2nimxa5jl-profile/sbin"
after:
# export PATH=\
"/gnu/store/6r16pi4zsxdf4a9p4v7padp2nimxa5jl-profile/bin\
:/gnu/store/6r16pi4zsxdf4a9p4v7padp2nimxa5jl-profile/sbin"
What I don't like, is when I switch to another mode (like M-x sh-mode
),
all this indentation stays the same. I think it is uncustomary when a
major mode changes the text and leaves it after it is disabled. Ideally
(and desirably) this text modification should be undone. Would it be
possible to do?
Also I don't like the keys you bind in guix-env-var-mode-map
: the
buffer with guix-env-var-mode
is not necessary read-only, so if I
decide to modify it, I will be very unpleased when instead of inserting
"n" key, the point is moved to the next paragraph. So I think, these
single keys shouldn't be bound.
Regarding auto-mode-alist
: do you think guix-env-var-mode
should
be enalbed by default in "/tmp/.../environment-variables" files? (I
think it should). If so, you need to add the ;;;###autoload
cookie to
the auto-mode-alist
line. Also in this case it doesn't make sense to
make guix-env-var-file-regexp
defcustom, as the user won't have a
chance to customize this variable because this auto-mode setting will be
autoloaded before the user's emacs config file.
Actually, because of that, I think there is no reason to make such file-regexp variables, and I just put all the auto-mode stuff in "guix-auto-mode.el" file. If a user wishes to override all these auto-mode settings, he/she can always do it in the emacs config.
Also, don't forget to add "guix-env-var.el" to "local.mk" file.
Thank you!
What I don't like, is when I switch to another mode (like
M-x sh-mode
), all this indentation stays the same.
I have suddenly recalled that there is guix-scheme-mode
that does a similar thing (for such files as /var/guix/profiles/system/boot
). I also didn't like this behaviour at that time, that's why I added a message "This buffer has been indented by
guix-scheme-mode'."`
Now I see, that it is problematic to restore the original indentation, so I think it's not worth borthering. But adding a similar message (so that a user can see that something happened with the "real" contents of the buffer) would be good. And maybe even adding a variable (like guix-env-var-enable-formatting
) that will control whether a buffer will be indented by default or not.
Hello Alex,
Apologies for a long reply.
I applied all your suggestions and pushed a commit. Please, take a look on it.
Thanks, Oleg.
A patch to simplify guix-env-var-prettify-variable
procedure:
diff --git a/elisp/guix-env-var.el b/elisp/guix-env-var.el
index 4a83373..ef4ed72 100644
--- a/elisp/guix-env-var.el
+++ b/elisp/guix-env-var.el
@@ -84,12 +84,11 @@ indented (while the original buffer file may not be indented).")
(when (search-forward "=" nil t)
(insert "\\")
(newline)
- (while (search-forward ":" nil t)
- (when (char-equal ?/ (following-char))
- (backward-char)
- (insert "\\")
- (newline)
- (forward-char))))
+ (while (search-forward ":/" nil t)
+ (backward-char 2)
+ (insert "\\")
+ (newline)
+ (forward-char)))
(end-of-line)
(widen))))
My main question is: I don't understand why you use guix-env-var-derived-sh-modes, guix-env-var-mode-eq and guix-env-var-mode-p.
Let's take a piece of /etc/profile
as an example:
# Crucial variables that could be missing in the profiles' 'etc/profile'
# because they would require combining both profiles.
# FIXME: See <http://bugs.gnu.org/20255>.
export MANPATH=$HOME/.guix-profile/share/man:/run/current-system/profile/share/man
export INFOPATH=$HOME/.guix-profile/share/info:/run/current-system/profile/share/info
export XDG_DATA_DIRS=$HOME/.guix-profile/share:/run/current-system/profile/share
export XDG_CONFIG_DIRS=$HOME/.guix-profile/etc/xdg:/run/current-system/profile/etc/xdg
Evaluation of (search-forward-regexp guix-env-var-regexp nil t)
procedure in a fundamental-mode
will move a cursor from a beginning of a buffer to (a pipe symbol):
export MANPATH=|$HOME/.guix-profile/share/man:/run/current-system/profile/share/man
but in sh-mode
the cursor will be one character behind:
export MANPATH|=$HOME/.guix-profile/share/man:/run/current-system/profile/share/man
Are they really needed?
I wrote a first implementation without sh-mode
derivation and found this issue after making guix-env-var-mode
derived from sh-mode
. guix-env-var-prettify-buffer
will fail in case when you use call it directly without guix-env-var-mode
.
Why do you autoload guix-env-var-prettify-variable and guix-env-var-prettify-buffer? Also why guix-env-var-prettify-variable is interactive? Is it intended for users?
Yes, both procedures are indented for users and autoloaded to make them available after starting Emacs. I'm sorry about forgetting to make guix-env-var-prettify-buffer
interactive.
I applied all your suggestions and pushed a new commit.
Evaluation of
(search-forward-regexp guix-env-var-regexp nil t)
procedure in afundamental-mode
will move a cursor from a beginning of a buffer to (a pipe symbol): [...] but insh-mode
the cursor will be one character behind:
Ah, that's because modes define symbol syntax differently (and I think fundamental-mode does not define it at all). What about fixing regexp instead? (I really think that this "mode-checking" workaround is the wrong way). For example, like this:
(defvar guix-env-var-regexp
(rx "export" (one-or-more space)
letter (zero-or-more (or alphanumeric "_")))
"Regexp matching shell variables in Guix environment-variables file.")
BTW I think (char space)
is not correct as potentially there may be several spaces after export
, so I changed it to (one-or-more space)
. WDYT?
Apologies for the last push, I was hurry and didn't check it properly. Don't look on this, because it's broken.
Actually it was OK, except I forgot to remove functions related to major mode.
I've applied all your suggestions. Thank you.
Also I found a bug with guix-env-var-prettify-buffer
procedure didn't prettify from a start of a buffer but from a current cursor position. Fixed in the last pushed commit.
Great, I made some minor changes and applied as commit 87a8c12210e8c3b853f95900eae72fb13411a130, thank you!
Oh, also I plan to make a release soon (hopefully soon enough :-)), is there anything you are willing to add/modify before the release?
[…]
is there anything you are willing to add/modify before the release?
I have some useful things in my .emacs
which users could find useful.
backward-paragraph
and forward-paragraph
in shell
First of all for guix package --search=git
inside M-x shell
Snippet, which make a use of backward-paragraph
and forward-paragraph
procedures easier.
(add-hook 'shell-mode-hook
(lambda ()
(progn (setq paragraph-separate "[ ]*$")
(setq paragraph-start "\\|[ ]*$"))))
For example, without the snippet the cursor after invoking backward-paragraph
two times, will be at the following line:
+ <http://dblp.uni-trier.de/search/>)
in:
guix package --search=git
[…]
+ <http://dblp.uni-trier.de/search/>)
+
+ * To check whether an article is freely available online, use ‘x’ in the list of
+ results. For example ‘M-x crossref-lookup RET Emacs stallman RET’ followed by ‘x
+ Dissemin RET’ will help you find open access copies of Stallman's paper on EMACS
+ (spoiler: http://hdl.handle.net/1721.1/5736).
+
+ See http://github.com/cpitclaudel/biblio.el for more information, including
+ documentation on extending this framework.
relevance: 2
[more packages …]
name: audacity
[more audacity fields…]
description: Audacity is a multi-track audio editor designed for recording, playing
+ and editing digital audio. It features digital effects and spectrum analysis
+ tools.
relevance: 2
name: alsa-modular-synth
version: 2.1.2
outputs: out
systems: x86_64-linux i686-linux armhf-linux aarch64-linux
dependencies: alsa-lib-1.1.5 clalsadrv-2.0.0 fftw-3.3.5 jack-0.125.0 ladspa-1.13
+ liblo-0.29 pkg-config-0.29.2 qtbase-5.9.4 qttools-5.9.4
location: gnu/packages/audio.scm:97:2
homepage: http://alsamodular.sourceforge.net/
license: GPL 2
synopsis: Realtime modular synthesizer and effect processor
description: AlsaModularSynth is a digital implementation of a classical analog
+ modular synthesizer system. It uses virtual control voltages to control the
+ parameters of the modules. The control voltages which control the frequency e.g.
+ of the VCO (Voltage Controlled Oscillator) and VCF (Voltage Controlled Filter)
+ modules follow the convention of 1V / Octave.
relevance: 2
natsu@magnolia ~$
but with a snippet the cursor will jump between empty lines (one line above name:
).
I have an advised find-file-at-point
procedure, which allows to call ffap
on:
/gnu/store/…-package-version
to invoke find /gnu/store/…-package-version
in shell
.DOCUMENTATION.info
or DOCUMENTATION.info.gz
to invoke (info DOCUMENTATION)
.man
pages./gnu/store/…-package-version.tar.gz
to invoke tar xf /gnu/store/…-package-version.tar.gz
in shell
.Procedure which opens a selected email in Gnus *Summary*
buffer in an Emacs
debbugs.
Procedure to copy a HTML URL to current selected node in Info
.
Procedure to show a commit hash at point in Magit in Guix repository.
Snippet, which make a use of backward-paragraph and forward-paragraph procedures easier.
(add-hook 'shell-mode-hook (lambda () (progn (setq paragraph-separate "[ ]*$") (setq paragraph-start "\\|[ ]*$"))))
Hm, I think this is more a general thing than a guix-specific setting.
I have an advised find-file-at-point procedure, which allows to call ffap on:
/gnu/store/…-package-version
to invoke find/gnu/store/…-package-version
in shell./gnu/store/…-package-version.tar.gz
to invoketar xf /gnu/store/…-package-version.tar.gz
in shell.
To be honest, I think these are personal settings, that people do in their emacs config files.
DOCUMENTATION.info or DOCUMENTATION.info.gz to invoke (info DOCUMENTATION).
Same for man pages. [...] Misc
Procedure which opens a selected email in Gnus Summary buffer in an Emacs debbugs.
Procedure to copy a HTML URL to current selected node in Info.
Procedure to show a commit hash at point in Magit in Guix repository.
These are also general things, not specific to guix. BTW, regarding info
: if you use (require 'dired-x)
, you can press I
in a dired buffer to invoke info
on the current file.
Sorry, if I wasn't polite, I really think these settings are just some personal preferences and customizations. My .emacs
is full of similar hacky things, but I doubt they should be put in a package form. So I think they probably shouldn't go to Emacs-Guix, but it's worth mentioning on a guix mailing list if you want to share your knowledge.
Anyway, thanks!
Thank you for pointing onto dired-x
package! I'll close the pull request.
Hello Alex,
This patch adds a functionality to prettify files like
/tmp/guix-build-hello-2.10.drv-0/environment-variables
. For example,guix build --no-grafts --check -K hello
will produceenvironment-variables
which in Emacs look as: