diff mbox series

[bug#66562,v3] gnu: emacs-haskell-snippets: Use correct directory for snippets.

Message ID 36c6ef9a672215883a84b8fec5a58a440d32ee11.1697394827.git.liliana.prikler@gmail.com
State New
Headers show
Series [bug#66562,v3] gnu: emacs-haskell-snippets: Use correct directory for snippets. | expand

Commit Message

Liliana Marie Prikler Oct. 15, 2023, 6:25 p.m. UTC
This package instructs Yasnippet to look for snippets relative to its own
path.  However, it installs the snippets directly to site-lisp, which as of
79cfe30f3eb10bd3dbf7aa0f6e873c945d7d0ea5 is one directory above that.  Use the
elpa-directory procedure introduced in that commit to rectify this mistake.

* gnu/packages/emacs-xyz.scm (emacs-haskell-snippets)[#:phases]
<install-snippets>: Use elpa-directory.

Fixes: 66562
---
Hi Rostislav,

Am Sonntag, dem 15.10.2023 um 17:17 +0200 schrieb Rostislav Svoboda:
> * gnu/packages/emacs-xyz.scm (emacs-haskell-snippets): Fix
> haskell-snippets-dir value
> 
> The value of haskell-snippets-dir was:
>   /gnu/store/*-emacs-haskell-snippets-*/share/emacs/site-lisp/haskell-snippets-*/
> It needs to be:
>   /gnu/store/*-emacs-haskell-snippets-*/share/emacs/site-lisp/
Actually, emacs-haskell-snippets is doing something wrong when installing files
directy to site-lisp instead of any other directory.  Here's an attempt to fix
that.

Cheers

 gnu/packages/emacs-xyz.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


base-commit: ef026e68fe58ed3be216c76f7c5f008893ed3095

Comments

Rostislav Svoboda Oct. 16, 2023, 10:57 a.m. UTC | #1
Hi Liliana,

Your patch works (thank you) and I improved it a tiny bit. (See
attachment.) BTW shouldn't the revision number in the (git-version
"0.1.0" "1" commit) be increased to "2" in your patch and to "3" in
mine?

Cheers

Le dim. 15 oct. 2023 à 20:36, Liliana Marie Prikler
<liliana.prikler@gmail.com> a écrit :
>
> This package instructs Yasnippet to look for snippets relative to its own
> path.  However, it installs the snippets directly to site-lisp, which as of
> 79cfe30f3eb10bd3dbf7aa0f6e873c945d7d0ea5 is one directory above that.  Use the
> elpa-directory procedure introduced in that commit to rectify this mistake.
>
> * gnu/packages/emacs-xyz.scm (emacs-haskell-snippets)[#:phases]
> <install-snippets>: Use elpa-directory.
>
> Fixes: 66562
> ---
> Hi Rostislav,
>
> Am Sonntag, dem 15.10.2023 um 17:17 +0200 schrieb Rostislav Svoboda:
> > * gnu/packages/emacs-xyz.scm (emacs-haskell-snippets): Fix
> > haskell-snippets-dir value
> >
> > The value of haskell-snippets-dir was:
> >   /gnu/store/*-emacs-haskell-snippets-*/share/emacs/site-lisp/haskell-snippets-*/
> > It needs to be:
> >   /gnu/store/*-emacs-haskell-snippets-*/share/emacs/site-lisp/
> Actually, emacs-haskell-snippets is doing something wrong when installing files
> directy to site-lisp instead of any other directory.  Here's an attempt to fix
> that.
>
> Cheers
>
>  gnu/packages/emacs-xyz.scm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index bb8c25f9b5..7f55febfbb 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -35546,10 +35546,11 @@ (define-public emacs-haskell-snippets
>          #:phases
>          #~(modify-phases %standard-phases
>              (add-after 'install 'install-snippets
> -              (lambda _
> +              (lambda* (#:key outputs #:allow-other-keys)
>                  (let ((snippets
>                         (string-append
> -                        #$output "/share/emacs/site-lisp/snippets/haskell-mode")))
> +                        (elpa-directory (assoc-ref outputs "out"))
> +                        "/snippets/haskell-mode")))
>                    (mkdir-p snippets)
>                    (copy-recursively "snippets/haskell-mode" snippets)))))))
>        (propagated-inputs
>
> base-commit: ef026e68fe58ed3be216c76f7c5f008893ed3095
> --
> 2.41.0
>
Liliana Marie Prikler Oct. 16, 2023, 4:57 p.m. UTC | #2
Am Montag, dem 16.10.2023 um 12:57 +0200 schrieb Rostislav Svoboda:
> Hi Liliana,
> 
> Your patch works (thank you) and I improved it a tiny bit. (See
> attachment.) BTW shouldn't the revision number in the (git-version
> "0.1.0" "1" commit) be increased to "2" in your patch and to "3" in
> mine?
No.  It should just be one patch anyway and the change doesn't affect
the source code, but the build recipe.  As such, the rebuild is going
to happen either way.

> DRY in the specification of the relative path of the snippets
> directory.
You can just amend my commit.

> -                (let ((snippets
> -                       (string-append
> -                        (elpa-directory (assoc-ref outputs "out"))
> -                        "/snippets/haskell-mode")))
> -                  (mkdir-p snippets)
> -                  (copy-recursively "snippets/haskell-mode"
> snippets)))))))
> +                (let* ((relative-dirpath "snippets/haskell-mode")
> +                       (installation-dir
> +                        (string-append (elpa-directory (assoc-ref
> outputs "out"))
> +                                       "/" relative-dirpath)))
> +                  (mkdir-p installation-dir)
> +                  (copy-recursively relative-dirpath installation-
> dir)))))))
Now you repeat yourself on relative-dirpath (which is a very Java name
anyway, just five characters shorter than the original value won't win
you Kolmogorov complexity).  Plus you're requiring let* instead of let.

Btw. don't 
  ((compose 
    (lambda (src dst) (mkdir-p src) (copy-recursively dst src))
    (lambda (dir store) (values dir (string-append store "/" dir)))
   "snippets/haskell-mode" (elpa-directory (assoc-ref outputs "out")))
to avoid gratuitous repetition.

Sometimes explicit is better than implicit, even if it comes at the
cost of typing a constant twice :)

Cheers
Rostislav Svoboda Oct. 17, 2023, 4:49 p.m. UTC | #3
Hi Liliana,

>> Your patch works (thank you) and I improved it a tiny bit. (See
>> attachment.) BTW shouldn't the revision number in the (git-version
>> "0.1.0" "1" commit) be increased to "2" in your patch and to "3" in
>> mine?
> No.  It should just be one patch anyway and the change doesn't affect
> the source code, but the build recipe.  As such, the rebuild is going
> to happen either way.
>
>> DRY in the specification of the relative path of the snippets
>> directory.
> You can just amend my commit.
>
>> -                (let ((snippets
>> -                       (string-append
>> -                        (elpa-directory (assoc-ref outputs "out"))
>> -                        "/snippets/haskell-mode")))
>> -                  (mkdir-p snippets)
>> -                  (copy-recursively "snippets/haskell-mode"
>> snippets)))))))
>> +                (let* ((relative-dirpath "snippets/haskell-mode")
>> +                       (installation-dir
>> +                        (string-append (elpa-directory (assoc-ref
>> outputs "out"))
>> +                                       "/" relative-dirpath)))
>> +                  (mkdir-p installation-dir)
>> +                  (copy-recursively relative-dirpath installation-
>> dir)))))))

> Now you repeat yourself on relative-dirpath

Sure. We have to specify what to copy, and where to copy it. If what
and where maintain the same structure, then some repetition is
inevitable.

> Sometimes explicit is better than implicit,
> even if it comes at the cost of typing a constant twice :)

A typo in a constant is a runtime error, whereas a typo in a variable
name gets caught by the compiler. That's the main rationale behind my
patch.

(The rest of my email contains just some side remarks.)

Cheers, Bost


> (which is a very Java name anyway, just five characters shorter than
> the original value won't win you Kolmogorov complexity).

If it were up to me, I'd use 'src', 'dst'. I find the 'no
abbreviations for identifiers' policy excessive.

> Plus you're requiring let* instead of let.

Having both variants is a language deficiency, in my opinion. Only let
should exist, functioning as let* does. This should extend to lambda*,
define*, etc.

> Btw. don't
>   ((compose
>     (lambda (src dst) (mkdir-p src) (copy-recursively dst src))
>     (lambda (dir store) (values dir (string-append store "/" dir)))
>    "snippets/haskell-mode" (elpa-directory (assoc-ref outputs "out")))
> to avoid gratuitous repetition.

On the one hand, we face gratuitous repetition; on the other, a
snippet like this better expresses compositional transformation
between inputs and outputs, which I find to be a way more important
that avoiding gratuitous repetition (pun intended). And as a side
effect it also simplifies debugging:

((compose
  ;; (lambda (a b) (format #t "[DBG] 3. a: ~a; b: ~a\n" a b) (values a b))
  (lambda (src dst) (mkdir-p src) (copy-recursively src dst))
  ;; (lambda (a b) (format #t "[DBG] 2. a: ~a; b: ~a\n" a b) (values a b))
  (lambda (dir store) (values dir (string-append store "/" dir)))
  ;; (lambda (a b) (format #t "[DBG] 1. a: ~a; b: ~a\n" a b) (values a b))
  )
 "snippets/haskell-mode" (elpa-directory (assoc-ref outputs "out")))

And if you insist, the gratuitous repetition could, in theory, be avoided:

((compose
  copy-recursively
  (juxt mkdir-p (partial string-append (elpa-directory (assoc-ref
outputs "out")) "/")))
 "snippets/haskell-mode")

Only if partial and juxt would exist... and here you go ;-)

(define (partial fun . args)
  "Partial function application."
  (lambda x (apply fun (append args x))))

(define (juxt . fns)
  "Naive implementation. Inspired by Clojure's juxt.
((juxt a b c) x) => (list (a x) (b x) (c x))"
  (lambda args
    (map (lambda (fn) (apply fn args)) fns)))

Here yet another pattern appears, the map-reduce. Also the juxt
function just screams for "let's call the mkdir-p and (partial
string-append ...) in parallel".

> Btw. don't (compose ...)

Quite the contrary, I think we should do more of (compose ...),
however functional composition is hard-to-impossible if e.g.:

- essential higher order functions like juxt and partial are not available.

- mkdir-p and copy-recursively from the (guix build utils) aren't
monadic and mkdir-p returns #t instead of a path-string and
copy-recursively returns:

  scheme@(guile-user)> ,use (guix build utils)
  scheme@(guile-user)> (copy-recursively "/tmp/f1.txt" "/tmp/f2.txt")
  `/tmp/foo.txt' -> `/tmp/fox.txt'

  eeeh... what exactly is the return value of copy-recursively? Hmm.

- copy-recursively, although naturally a reducer (i.e. a member of the
fold-family, think of 'a list of things goes into a container') is not
implemented as such. Hmm, disappointing... although a <...>-fold is
used in its implementation. Double hmm.

- in general, the built-in compose function can't be called with zero
arguments. For that purpose I cobbled myself:

(define (comp . fns)
  "Like `compose'. Can be called with zero arguments. I.e. (thunk? comp) => #t
Works also for functions returning and accepting multiple values."
  (lambda args
    (if (null? fns)
        (apply values args)
        (let [(proc (car fns)) (rest (cdr fns))]
          (if (null? rest)
              (apply proc args)
              (let ((g (apply comp rest)))
                (call-with-values (lambda () (apply g args)) proc)))))))


And finally, in the (guix build utils) there's the install-file which
works with single files. What about adding its recursive version:

(define* (install-recursively source destination
                              #:key
                              (log (current-output-port))
                              (follow-symlinks? #f)
                              (copy-file copy-file)
                              keep-mtime? keep-permissions?)
  "Recursive version of install-file."
  (mkdir-p destination)
  (copy-recursively source
                    (string-append destination "/" (basename destination))
                    #:log log
                    #:follow-symlinks? follow-symlinks?
                    #:copy-file copy-file
                    #:keep-mtime? keep-mtime?
                    #:keep-permissions? keep-permissions?))
Liliana Marie Prikler Oct. 17, 2023, 5:29 p.m. UTC | #4
Hi, Rostislav

Am Dienstag, dem 17.10.2023 um 18:49 +0200 schrieb Rostislav Svoboda:
> […]
> > Plus you're requiring let* instead of let.
> 
> Having both variants is a language deficiency, in my opinion. Only
> let should exist, functioning as let* does. This should extend to
> lambda*, define*, etc.
Only let should exist, functioning as let does.  let* is sugar on top.
(Not serious about this, there are reasons to have let*, but there are
also reasons it's not the default behaviour.)

> > Btw. don't
> >   ((compose
> >     (lambda (src dst) (mkdir-p src) (copy-recursively dst src))
> >     (lambda (dir store) (values dir (string-append store "/" dir)))
> >    "snippets/haskell-mode" (elpa-directory (assoc-ref outputs
> > "out")))
> > to avoid gratuitous repetition.
> 
> On the one hand, we face gratuitous repetition; on the other, a
> snippet like this better expresses compositional transformation
> between inputs and outputs, which I find to be a way more important
> that avoiding gratuitous repetition (pun intended). And as a side
> effect it also simplifies debugging:
> 
> ((compose
>   ;; (lambda (a b) (format #t "[DBG] 3. a: ~a; b: ~a\n" a b) (values
> a b))
>   (lambda (src dst) (mkdir-p src) (copy-recursively src dst))
>   ;; (lambda (a b) (format #t "[DBG] 2. a: ~a; b: ~a\n" a b) (values
> a b))
>   (lambda (dir store) (values dir (string-append store "/" dir)))
>   ;; (lambda (a b) (format #t "[DBG] 1. a: ~a; b: ~a\n" a b) (values
> a b))
>   )
>  "snippets/haskell-mode" (elpa-directory (assoc-ref outputs "out")))
If you need to warp your head around through three debug statements,
I'm not convinced you're improving the overall code all that much.

> And if you insist, the gratuitous repetition could, in theory, be
> avoided:
> 
> ((compose
>   copy-recursively
>   (juxt mkdir-p (partial string-append (elpa-directory (assoc-ref
> outputs "out")) "/")))
>  "snippets/haskell-mode")
> 
> Only if partial and juxt would exist... and here you go ;-)
> 
> (define (partial fun . args)
>   "Partial function application."
>   (lambda x (apply fun (append args x))))
> 
> (define (juxt . fns)
>   "Naive implementation. Inspired by Clojure's juxt.
> ((juxt a b c) x) => (list (a x) (b x) (c x))"
>   (lambda args
>     (map (lambda (fn) (apply fn args)) fns)))
> 
> Here yet another pattern appears, the map-reduce. Also the juxt
> function just screams for "let's call the mkdir-p and (partial
> string-append ...) in parallel".
You can do all that, but it does go against the KISS principle :)

> > Btw. don't (compose ...)
> 
> Quite the contrary, I think we should do more of (compose ...),
> however functional composition is hard-to-impossible if e.g.:
> 
> - essential higher order functions like juxt and partial are not
> available.
We have partial.  We call it cut.  It's part of SRFI-26.
It even simplifies the definition of juxt: 
(lambda args (map (cut apply <> args) fns))
Anyhow, we quite often don't use it because we'd have to add it to
#:modules and the benefit over raw lambdas is often negligible.

> - mkdir-p and copy-recursively from the (guix build utils) aren't
> monadic and mkdir-p returns #t instead of a path-string and
> copy-recursively returns:
> 
>   scheme@(guile-user)> ,use (guix build utils)
>   scheme@(guile-user)> (copy-recursively "/tmp/f1.txt" "/tmp/f2.txt")
>   `/tmp/foo.txt' -> `/tmp/fox.txt'
> 
>   eeeh... what exactly is the return value of copy-recursively? Hmm.
It returns *unspecified*.  Yes, most of this stuff is indeed not
monadic.  Scheme is not purely functional, so side effects are allowed
:)

> - copy-recursively, although naturally a reducer (i.e. a member of
> the fold-family, think of 'a list of things goes into a container')
> is not implemented as such. Hmm, disappointing... although a <...>-
> fold is used in its implementation. Double hmm.
There is a cost to constructing the return value of a fold.  I
personally can do without creating lists that no one will end up
inspecting anyway.

> - in general, the built-in compose function can't be called with zero
> arguments. For that purpose I cobbled myself:
> 
> (define (comp . fns)
>   "Like `compose'. Can be called with zero arguments. I.e. (thunk?
> comp) => #t
> Works also for functions returning and accepting multiple values."
>   (lambda args
>     (if (null? fns)
>         (apply values args)
>         (let [(proc (car fns)) (rest (cdr fns))]
>           (if (null? rest)
>               (apply proc args)
>               (let ((g (apply comp rest)))
>                 (call-with-values (lambda () (apply g args))
> proc)))))))
I'd argue that compose without procedures is quite meaningless, but
maybe I'm thinking too hard.

> And finally, in the (guix build utils) there's the install-file which
> works with single files. What about adding its recursive version:
> 
> (define* (install-recursively source destination
>                               #:key
>                               (log (current-output-port))
>                               (follow-symlinks? #f)
>                               (copy-file copy-file)
>                               keep-mtime? keep-permissions?)
>   "Recursive version of install-file."
>   (mkdir-p destination)
>   (copy-recursively source
>                     (string-append destination "/" (basename
> destination))
>                     #:log log
>                     #:follow-symlinks? follow-symlinks?
>                     #:copy-file copy-file
>                     #:keep-mtime? keep-mtime?
>                     #:keep-permissions? keep-permissions?))
There'd be no point in having copy-recursively then.  For a more
complete build system that already takes care of all that without
having you fiddling with juxt, partial, etc., just take copy-build-
system.  Not that it's needed here, mind you.

Cheers
Rostislav Svoboda Oct. 18, 2023, 8:54 a.m. UTC | #5
Hi Liliana,

I think we're digressing and since our discussion is mostly about
opinions I'd prefer to discontinue it. Except for one rather important
point:

> > - in general, the built-in compose function can't be called with zero
> > arguments. For that purpose I cobbled myself:
...
> I'd argue that compose without procedures is quite meaningless, but
> maybe I'm thinking too hard.

Food for thought:

in Emacs Lisp:
*** Welcome to IELM ***  Type (describe-mode) or press C-h m for help.
ELISP> (-compose)
#<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_131>

In Racket:
Welcome to Racket v8.10 [cs].
> (compose)
#<procedure:values>

Clojure 1.11.1
user=> (comp)
#object[clojure.core$identity 0x610db97e "clojure.core$identity@610db97e"]

Cheers
Liliana Marie Prikler Oct. 18, 2023, 9:58 p.m. UTC | #6
Am Mittwoch, dem 18.10.2023 um 10:54 +0200 schrieb Rostislav Svoboda:
> Hi Liliana,
> 
> I think we're digressing and since our discussion is mostly about
> opinions I'd prefer to discontinue it. 
Agree to disagreeing then.

> Except for one rather important point:
> 
> > > - in general, the built-in compose function can't be called with
> > > zero arguments. For that purpose I cobbled myself:
> ...
> > I'd argue that compose without procedures is quite meaningless, but
> > maybe I'm thinking too hard.
> 
> Food for thought:
> 
> in Emacs Lisp:
> *** Welcome to IELM ***  Type (describe-mode) or press C-h m for
> help.
> ELISP> (-compose)
> #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_131>
> 
> In Racket:
> Welcome to Racket v8.10 [cs].
> > (compose)
> #<procedure:values>
> 
> Clojure 1.11.1
> user=> (comp)
> #object[clojure.core$identity 0x610db97e
> "clojure.core$identity@610db97e"]
Looks like a bunch of implementation-defined behaviour to me.  Note
that rnrs barely specifies the case for two arguments :)

Cheers
Liliana Marie Prikler Oct. 23, 2023, 7:58 a.m. UTC | #7
Am Sonntag, dem 15.10.2023 um 20:25 +0200 schrieb Liliana Marie
Prikler:
> This package instructs Yasnippet to look for snippets relative to its
> own
> path.  However, it installs the snippets directly to site-lisp, which
> as of
> 79cfe30f3eb10bd3dbf7aa0f6e873c945d7d0ea5 is one directory above
> that.  Use the
> elpa-directory procedure introduced in that commit to rectify this
> mistake.
> 
> * gnu/packages/emacs-xyz.scm (emacs-haskell-snippets)[#:phases]
> <install-snippets>: Use elpa-directory.
> 
> Fixes: 66562
> ---
Made the fixes line more descriptive and pushed.

Cheers
diff mbox series

Patch

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index bb8c25f9b5..7f55febfbb 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -35546,10 +35546,11 @@  (define-public emacs-haskell-snippets
         #:phases
         #~(modify-phases %standard-phases
             (add-after 'install 'install-snippets
-              (lambda _
+              (lambda* (#:key outputs #:allow-other-keys)
                 (let ((snippets
                        (string-append
-                        #$output "/share/emacs/site-lisp/snippets/haskell-mode")))
+                        (elpa-directory (assoc-ref outputs "out"))
+                        "/snippets/haskell-mode")))
                   (mkdir-p snippets)
                   (copy-recursively "snippets/haskell-mode" snippets)))))))
       (propagated-inputs