[bug#67019] [PATCH 03/16] gnu: Add lessc.

[Philip McGrath]

Hi,

On 11/15/23 15:23, Liliana Marie Prikler wrote:
> Am Mittwoch, dem 15.11.2023 um 14:35 -0500 schrieb Philip McGrath:
> > On 11/10/23 19:56, Liliana Marie Prikler wrote:
> > > Am Donnerstag, dem 09.11.2023 um 11:26 -0500 schrieb Philip
> > > McGrath:
> > > > * gnu/packages/web.scm (lessc): New variable.
> > > >
> > > > [...]
> >   >>
> > > > +          (add-after 'avoid-parse-node-version 'do-not-target-
> > > > es5
> > > > +            (lambda args
> > > > +              ;; esbuild can't compile all features to ES5
> > > > +              (with-atomic-json-file-replacement "tsconfig.json"
> > > > +                (match-lambda
> > > > +                  (('@ . alist)
> > > > +                   (cons '@
> > > > +                    (map (match-lambda
> > > > +                           (("compilerOptions" '@ . alist)
> > > > +                            `("scripts" @ ,@(filter (match-
> > > > lambda
> > > > +                                                      (("target"
> > > > "ES5")
> > > > +                                                       #f)
> > > > +                                                      (_
> > > > +                                                       #t))
> > > > +                                                    alist)))
> > > > +                           (other
> > > > +                            other))
> > > > +                         alist)))))))
> > > > +          (add-after 'do-not-target-es5 'patch-build-script
> > > > +            (lambda args
> > > > +              (define new-build-script
> > > > +                (string-join
> > > > +                 `("esbuild"
> > > > +                   "--platform=node"
> > > > +                   "--format=cjs"
> > > > +                   "--outdir=lib"
> > > > +                   ,@(find-files "src/less" "\\.js$")
> > > > +                   ,@(find-files "src/less-node" "\\.js$"))))
> > > > +              (with-atomic-json-file-replacement "package.json"
> > > > +                (match-lambda
> > > > +                  (('@ . alist)
> > > > +                   (cons '@
> > > > +                    (map (match-lambda
> > > > +                           (("scripts" @ . alist)
> > > > +                            `("scripts" @ ,@(map (match-lambda
> > > > +                                                   (("build" .
> > > > _)
> > > > +                                                    (cons
> > > > "build"
> > > > +                                                          new-
> > > > build-
> > > > script))
> > > > +                                                   (other
> > > > +                                                    other))
> > > > +                                                 alist)))
> > > > +                           (other
> > > > +                            other))
> > > > +                         alist)))))))
> > > Can we somehow save a bit of horizontal real-estate here?  Same
> > > goes
> > > for 1 and 2.
> >
> > To clarify, do you mean vertical or horizontal?
> I do mean horizontal.
>
> [...]
>
> > I could also imagine breaking these lines:
> >
> >   >> +                           (("scripts" @ . alist)
> >   >> +                            `("scripts" @ ,@(map (match-lambda
> >
> > instead as:
> >
> >   >> +                           (("scripts"
> >   >> +                             @ . alist)
> >   >> +                            `("scripts"
> >   >> +                              @ ,@(map (match-lambda
> >
> > but that doesn't seem like much of an improvement to me.
> But what about breaking lines before (match-lambda?  That ought to at
> least give us enough to get (_ #f) onto a single line, no?

Maybe I'm confused: there isn't (_ #f) anywhere. There is currently 
enough space to put (other other) on a single line, but I thought it was 
better style to put a newline between the match pattern and the 
expression, especially when the pattern is not _.

Breaking before match-lambda gets enough space to put (cons "build" 
new-build-script) on a single line, but I don't think it looks better 
overall:

>           (add-after 'do-not-target-es5 'patch-build-script
>             (lambda args
>               (define new-build-script
>                 (string-join
>                  `("esbuild"
>                    "--platform=node"
>                    "--format=cjs"
>                    "--outdir=lib"
>                    ,@(find-files "src/less" "\\.js$")
>                    ,@(find-files "src/less-node" "\\.js$"))))
>               (with-atomic-json-file-replacement "package.json"
>                 (match-lambda
>                   (('@ . alist)
>                    (cons '@
>                     (map
>                      (match-lambda
>                        (("scripts" @ . alist)
>                         `("scripts" @ ,@(map
>                                          (match-lambda
>                                            (("build" . _)
>                                             (cons "build" new-build-script))
>                                            (other
>                                             other))
>                                          alist)))
>                        (other
>                         other))
>                      alist)))))))

Using delete in do-not-target-es5 does seem like a minor improvement:

>           (add-after 'avoid-parse-node-version 'do-not-target-es5
>             (lambda args
>               ;; esbuild can't compile all features to ES5
>               (with-atomic-json-file-replacement "tsconfig.json"
>                 (match-lambda
>                   (('@ . alist)
>                    (cons '@
>                     (map (match-lambda
>                            (("compilerOptions" '@ . alist)
>                             `("scripts" @ ,@(delete '("target" "ES5")
>                                                     alist)))
>                            (other
>                             other))
>                          alist)))))))

Philip