Re: [PATCH] Automatically set `geiser-guile-load-path' from .dir-locals

[Miguel Ángel Arruga Vivas]

Hi Christopher,

First of all I want to say sorry: I've tested this and reviewed the
patch, and this is the second issue that already has caused, so yes, my
tests weren't enough at all.

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

> Also, I hope this email isn't interpreted as being dismissive or negative
> about what looks like it's probably a real usability improvement for
> hacking on Guix!  I just thought my experience indicated maybe there are
> additional considerations to address.

At least from my side I see your report as something positive, I cannot
see how could it be negative at all, and I'd like to thank you for it.

> Christopher Lemmer Webber writes:
>> I'm a bit unsure what the implications fully are with this change, but
>> here was my user experience:
>>
>>  - Did a pull using magit to guix

I use magit too, so I guess this isn't the source of the error.

>>  - Suddenly every file I open in Guix is prompting me if I want to
>>    enable these dir-locals, I notice some have "eval" and I don't know
>>    what it's doing so I say no

Saying no here shouldn't be a problem, as the variables should always be
optional.

>>  - But it's asking me every file

If every file means "every file on guix project" that should be the
normal behavior of Emacs for .dir-locals.el since this file was
introduced long before the patch: you can mark the 'eval' lines to be
accepted, or to be rejected always, but they're loaded with each file.
A problem could be that the UI shows the ones you have already accepted
too, and simply marks with * the new ones.

If it means "every other file too", I'm unable to reproduce that with a
fresh Emacs session.

>>  - And oh no, it's asking me about ten times for every magit operation!
>>    (Presumably due to the reload operation.)  Fine okay fine, YES, okay
>>    cool it's quiet now... I hope that was safe.

The only effects of the new code should be:

  * First eval: Set guix-directory for emacs-guix to the folder where
    .dir-locals.el is located.  This affects the whole emacs-guix, but
    AFAIU this isn't your issue.

  * Second eval:
    - Make geiser-guile-load-path buffer-local, and optionally define it
      as empty if it was void.
    - Add the directory where .dir-locals.el is located to
      geiser-guile-load-path.

>> Later...
>>
>>  - I'm hacking on another file in another repo
>>  - C-x v = (check to see a diff of the work-in-progress changes of the
>>    file I'm working on)

Thanks, with that I reproduce the problem, but I cannot debug it right
now.  I'll be available in some hours, as soon as I have anything I'll
update about this.

>>  - Error in the minibuffer!  "Wrong type argument: stringp, nil"
>>  - wtf, okay M-x toggle-debug-on-error
>>
>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>   expand-file-name(nil)
>>   (let* ((root-dir (expand-file-name (locate-dominating-file 
>> default-directory ".dir-locals.el"))) (root-dir* (directory-file-name 
>> root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar 
>> geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) 
>> (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test 
>> #'string-equal))
>>   eval((let* ((root-dir (expand-file-name (locate-dominating-file 
>> default-directory ".dir-locals.el"))) (root-dir* (directory-file-name 
>> root-dir))) (unless (boundp 'geiser-guile-load-path) (defvar 
>> geiser-guile-load-path 'nil)) (make-local-variable 'geiser-guile-load-path) 
>> (require 'cl-lib) (cl-pushnew root-dir* geiser-guile-load-path :test 
>> #'string-equal)))
>>   hack-one-local-variable(eval (let* ((root-dir (expand-file-name 
>> (locate-dominating-file default-directory ".dir-locals.el"))) (root-dir* 
>> (directory-file-name root-dir))) (unless (boundp 'geiser-guile-load-path) 
>> (defvar geiser-guile-load-path 'nil)) (make-local-variable 
>> 'geiser-guile-load-path) (require 'cl-lib) (cl-pushnew root-dir* 
>> geiser-guile-load-path :test #'string-equal)))
>>   hack-local-variables-apply()
>>   hack-dir-local-variables-non-file-buffer()
>>   diff-mode()
>>   vc-diff-internal(t (Git 
>> ("/home/cwebber/devel/scribble/scribble-lib/scribble...")) nil nil t)
>>   vc-diff(nil t)
>>   funcall-interactively(vc-diff nil t)
>>   call-interactively(vc-diff nil nil)
>>   command-execute(vc-diff)
>>  
>>  - Oh... it's whatever thing I enabled earlier in the guix repo.  Well
>>    now my vc-diff is broken outside of there... :\
>>
>> I'm presuming that if I understood whatever this is doing, it's probably
>> something that gives me a better user experience if I accept it while
>> working on Guix.  But a) for whatever reason Emacs' dir-locals stuff is
>> written in such a way that it antagonizes me for not accepting it and I
>> didn't know what it eval was (maybe this is a lack of understanding in
>> how to "say no and get it to listen to me"... I didn't resist for too
>> long) and b) it seems like this change isn't scoped to Guix's checkout
>> itself somehow...

Sorry again, as soon as I have a bit of time I'll update.

Happy hacking!
Miguel

signature.asc
Description: PGP signature