guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#66562] [PATCH v3] gnu: emacs-haskell-snippets: Use correct director


From: Rostislav Svoboda
Subject: [bug#66562] [PATCH v3] gnu: emacs-haskell-snippets: Use correct directory for snippets.
Date: Tue, 17 Oct 2023 18:49:23 +0200

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?))





reply via email to

[Prev in Thread] Current Thread [Next in Thread]