diff mbox series

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

Message ID ab477ad53c7277a9363eb5090493dd2bc81174c8.1699540553.git.philip@philipmcgrath.com
State New
Headers show
Series gnu: Add KaTeX, lessc, and flow-remove-types. | expand

Commit Message

Philip McGrath Nov. 9, 2023, 4:26 p.m. UTC
* gnu/packages/web.scm (lessc): New variable.
---
 gnu/packages/web.scm | 187 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 187 insertions(+)

Comments

Liliana Marie Prikler Nov. 11, 2023, 12:56 a.m. UTC | #1
Am Donnerstag, dem 09.11.2023 um 11:26 -0500 schrieb Philip McGrath:
> * gnu/packages/web.scm (lessc): New variable.
> ---
>  gnu/packages/web.scm | 187
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 187 insertions(+)
> 
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 66d09700db..8e22997957 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -64,6 +64,7 @@
>  ;;; Copyright © 2023 David Thompson <dthompson2@worcester.edu>
>  ;;; Copyright © 2023 Christopher Howard
> <christopher@librehacker.com>
>  ;;; Copyright © 2023 Felix Lechner <felix.lechner@lease-up.com>
> +;;; Copyright © 2023 Philip McGrath <philip@philipmcgrath.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -98,6 +99,7 @@ (define-module (gnu packages web)
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system go)
>    #:use-module (guix build-system meson)
> +  #:use-module (guix build-system node)
>    #:use-module (guix build-system perl)
>    #:use-module (guix build-system pyproject)
>    #:use-module (guix build-system python)
> @@ -168,6 +170,7 @@ (define-module (gnu packages web)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages networking)
>    #:use-module (gnu packages node)
> +  #:use-module (gnu packages node-xyz)
>    #:use-module (gnu packages nss)
>    #:use-module (gnu packages openldap)
>    #:use-module (gnu packages openstack)
> @@ -2343,6 +2346,190 @@ (define-public sassc/libsass-3.5
>                                                     
> "0830pjcvhzxh6yixj82x5k5r1xnadjqzi16kp53213icbly0r9ma")))))))
>      (properties '((hidden? . #t)))))
>  
> +(define-public lessc
> +  (package
> +    (name "lessc")
> +    (version "4.2.0")
> +    (source
> +     (origin (method git-fetch)
> +             (uri (git-reference
> +                   (url "https://github.com/less/less.js")
> +                   (commit (string-append "v" version))))
> +             (sha256
> +              (base32
> "1b6anlafk7lnayxy3vhsi474jcdah2ffaw2qyac5s2ibxb1wmr54"))
> +             (snippet
> +              #~(begin
> +                  (use-modules (guix build utils))
> +                  (delete-file-recursively "packages/less/dist")))
> +             (file-name (git-file-name name version))))
> +    (build-system node-build-system)
> +    (native-inputs (list esbuild))
> +    (inputs (list node-copy-anything))
> +    (arguments
> +     (list
> +      #:tests? #f ; many more dependencies
> +      #:modules
> +      `((guix build node-build-system)
> +        (ice-9 match)
> +        (guix build utils))
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'unpack 'chdir
> +            (lambda args
> +              (chdir "packages/less")))
> +          (add-after 'patch-dependencies 'delete-dependencies
> +            (lambda args
> +              (delete-dependencies
> +               '(;; dependencies
> +                 "parse-node-version" ; patched out
> +                 "tslib" ; probably not needed w/ esbuild
> +                 ;; devDependencies
> +                 "@less/test-data"
> +                 "@less/test-import-module"
> +                 "@rollup/plugin-commonjs"
> +                 "@rollup/plugin-json"
> +                 "@rollup/plugin-node-resolve"
> +                 "@typescript-eslint/eslint-plugin"
> +                 "@typescript-eslint/parser"
> +                 "benny"
> +                 "bootstrap-less-port"
> +                 "chai"
> +                 "cross-env"
> +                 "diff"
> +                 "eslint"
> +                 "fs-extra"
> +                 "git-rev"
> +                 "globby"
> +                 "grunt"
> +                 "grunt-cli"
> +                 "grunt-contrib-clean"
> +                 "grunt-contrib-connect"
> +                 "grunt-eslint"
> +                 "grunt-saucelabs"
> +                 "grunt-shell"
> +                 "html-template-tag"
> +                 "jit-grunt"
> +                 "less-plugin-autoprefix"
> +                 "less-plugin-clean-css"
> +                 "minimist"
> +                 "mocha"
> +                 "mocha-headless-chrome"
> +                 "mocha-teamcity-reporter"
> +                 "nock"
> +                 "npm-run-all"
> +                 "performance-now"
> +                 "phin"
> +                 "promise"
> +                 "read-glob"
> +                 "resolve"
> +                 "rollup"
> +                 "rollup-plugin-terser"
> +                 "rollup-plugin-typescript2"
> +                 "semver"
> +                 "shx"
> +                 "time-grunt"
> +                 "ts-node"
> +                 "typescript"
> +                 "uikit"
> +                 ;; optionalDependencies
> +                 "errno"
> +                 "graceful-fs"
> +                 "image-size"
> +                 "make-dir"
> +                 "mime"
> +                 "needle"
> +                 "source-map"))))
> +          (add-after 'delete-dependencies 'avoid-parse-node-version
> +            (lambda args
> +              (define version
> +                #$(package-version this-package))
> +              (substitute* "src/less/index.js"
> +                (("import [{] version [}]" orig)
> +                 (string-append "// " orig))
> +                (("import parseVersion from 'parse-node-version';"
> orig)
> +                 (string-append "// " orig))
> +                (("const v = parseVersion[(]`v\\$[{]version[}]`[)];"
> orig)
> +                 (string-append "// " orig))
> +                (("(version: )(\\[v\\.major, v\\.minor,
> v\\.patch],)" _ lhs rhs)
> +                 (string-append
> +                  lhs
> +                  "["
> +                  (string-join
> +                   (list-head (string-split
> +                               version (char-set-complement char-
> set:digit))
> +                              3)
> +                   ", ")
> +                  "], // "
> +                  rhs)))))
> +          (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.
> +          (add-after 'build 'build-browser
> +            (lambda args
> +              (invoke "esbuild"
> +                      "--bundle"
> +                      "--platform=browser"
> +                      "--format=cjs"
> +                      "--outfile=dist/less.js"
> +                      "src/less-browser/bootstrap.js")
> +              (invoke "esbuild"
> +                      "--bundle"
> +                      "--minify"
> +                      "--sourcemap"
> +                      "--platform=browser"
> +                      "--format=cjs"
> +                      "--outfile=dist/less.min.js"
> +                      "src/less-browser/bootstrap.js"))))))
> +    (home-page "https://lesscss.org")
> +    (synopsis "Compiler for @acronym{Less} @acronym{CSS} language
> extension")
> +    ;; XXX: @abbr{} seems better for Less (which is always
> capitalized that
> +    ;; way), but it is rejected as invalid Texinfo markup here.
> +    (description "@acronym{Less, Leaner Style Sheets} is a
> +backwards-compatible language extension for @acronym{CSS}.  This
> package
> +provides @command{lessc}, which compiles Less files to plain
> @acronym{CSS}.")
> +    (license license:asl2.0)))
> +
IMHO it doesn't make sense to type @acronym without the expansion.

Cheers
Philip McGrath Nov. 15, 2023, 7:35 p.m. UTC | #2
Hi,

Thanks for taking a look at this!

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?

The long list of dependencies to delete does take a *lot* of vertical 
space, especially in this patch. The obvious alternative would be to put 
more than one dependency on the same line. I'm not opposed to that, but 
it would deviate from normal style and could make future diffs less clear.

For horizontal space, I don't really like any of the alternatives I've 
thought of. The culprit in each case seems to be the three 
`match-lambda`s under `with-atomic-json-file-replacement`. (Specifically 
in do-not-target-es5, I guess the innermost one could be replaced with 
just `remove`, which might help a little.)

In normal programming, I'd want to abstract the three patch-build-script 
phases into a helper function that would take the new-build-script 
string as an argument, but that's a bit awkward to do with build-side 
code in Guix. Putting it in guix/build/node-build-system.scm (like 
delete-dependencies) would trigger a lot of rebuilds that seem hard to 
justify. It could be done as a gexp-producing function, but 
node-is-what, node-copy-anything, and lessc aren't in the same file: I 
guess the arguments field is delayed, so maybe it wouldn't create a 
cyclic dependency issue, but that seemed to open a whole new can of worms.

(Making e.g. `jsobject-update*` from guix/build/node-build-system.scm 
public would also help, but I have no desire to revisit that.)

Obviously it would be possible, within each G-expression, to lift one of 
the `match-lambda`s (probably the innermost one) to a local definition, 
but IMO that would make the structure of the code less obviously 
correspond to the structure of the JSON being transformed.

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.


>> +    (synopsis "Compiler for @acronym{Less} @acronym{CSS} language
>> extension")
>> +    ;; XXX: @abbr{} seems better for Less (which is always
>> capitalized that
>> +    ;; way), but it is rejected as invalid Texinfo markup here.
>> +    (description "@acronym{Less, Leaner Style Sheets} is a
>> +backwards-compatible language extension for @acronym{CSS}.  This
>> package
>> +provides @command{lessc}, which compiles Less files to plain
>> @acronym{CSS}.")
>> +    (license license:asl2.0)))
>> +
> IMHO it doesn't make sense to type @acronym without the expansion.
> 

I don't have a strong opinion on this, and I only know the tiny amount 
of Texinfo I've picked up for Guix. I inferred from the fact that the 
one-argument version of @acronym{} exists that it should be used when 
applicable. I know that some typographical conventions handle capital 
letters and punctuation in acronyms specially, which would be one reason 
for @acronym{} to exist, but maybe that isn't relevant?

Philip
Liliana Marie Prikler Nov. 15, 2023, 8:23 p.m. UTC | #3
Am Mittwoch, dem 15.11.2023 um 14:35 -0500 schrieb Philip McGrath:
> Hi,
> 
> Thanks for taking a look at this!
> 
> 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.

> The long list of dependencies to delete does take a *lot* of vertical
> space, especially in this patch. The obvious alternative would be to
> put  more than one dependency on the same line. I'm not opposed to
> that, but it would deviate from normal style and could make future
> diffs less clear.
Speaking of which, can we has regexps here?

> For horizontal space, I don't really like any of the alternatives
> I've thought of. The culprit in each case seems to be the three 
> `match-lambda`s under `with-atomic-json-file-replacement`.
> (Specifically in do-not-target-es5, I guess the innermost one could
> be replaced with just `remove`, which might help a little.)
> 
> In normal programming, I'd want to abstract the three patch-build-
> script phases into a helper function that would take the new-build-
> script string as an argument, but that's a bit awkward to do with
> build-side code in Guix. Putting it in guix/build/node-build-
> system.scm (like 
> delete-dependencies) would trigger a lot of rebuilds that seem hard
> to justify. It could be done as a gexp-producing function, but 
> node-is-what, node-copy-anything, and lessc aren't in the same file:
> I guess the arguments field is delayed, so maybe it wouldn't create a
> cyclic dependency issue, but that seemed to open a whole new can of
> worms.
> 
> (Making e.g. `jsobject-update*` from guix/build/node-build-system.scm
> public would also help, but I have no desire to revisit that.)
I think this is valid criticism of our node-build-system to perhaps
take to another thread.

> Obviously it would be possible, within each G-expression, to lift one
> of the `match-lambda`s (probably the innermost one) to a local
> definition, but IMO that would make the structure of the code less
> obviously correspond to the structure of the JSON being transformed.
> 
> 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?

> > > +    (synopsis "Compiler for @acronym{Less} @acronym{CSS}
> > > language
> > > extension")
> > > +    ;; XXX: @abbr{} seems better for Less (which is always
> > > capitalized that
> > > +    ;; way), but it is rejected as invalid Texinfo markup here.
> > > +    (description "@acronym{Less, Leaner Style Sheets} is a
> > > +backwards-compatible language extension for @acronym{CSS}.  This
> > > package
> > > +provides @command{lessc}, which compiles Less files to plain
> > > @acronym{CSS}.")
> > > +    (license license:asl2.0)))
> > > +
> > IMHO it doesn't make sense to type @acronym without the expansion.
> > 
> 
> I don't have a strong opinion on this, and I only know the tiny
> amount of Texinfo I've picked up for Guix. I inferred from the fact
> that the one-argument version of @acronym{} exists that it should be
> used when applicable. I know that some typographical conventions
> handle capital letters and punctuation in acronyms specially, which
> would be one reason for @acronym{} to exist, but maybe that isn't
> relevant?
Yeah, CAPS aren't relevant here.  The @acronym is typically for giving
meaning to an acronym, but if said meaning has already been given once,
repeating it would not make sense.

Some quotes from the manual:

>    - In Texinfo, an acronym (but not an abbreviation) should consist
>      only of capital letters and periods, no lowercase.
>    [...]
>    - It usually turns out to be quite difficult and/or time-consuming
> to consistently use '@acronym' for all sequences of uppercase
>      letters.  Furthermore, it looks strange for some acronyms to be
>      in the normal font size and others to be smaller.  Thus, a
>      simpler approach you may wish to consider is to avoid '@acronym'
>      and just typeset everything as normal text in all capitals:
>      'GNU', producing the output 'GNU'.
> 
>    - In general, it's not essential to use either of these commands
>      for all abbreviations; use your judgment.  Text is perfectly
>      readable without them.

Cheers
Philip McGrath Nov. 16, 2023, 12:03 a.m. UTC | #4
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
Liliana Marie Prikler Nov. 16, 2023, 1:17 a.m. UTC | #5
Hi,

Am Mittwoch, dem 15.11.2023 um 19:03 -0500 schrieb 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:
> > > [...]
> > > 
> > > 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 was a (_ #t) in the filter, though :)
In any case, such trivial matches should fit onto one line imho.

> 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
> _.
IMHO, this only makes sense for non-trivial replacements.  If you keep
some piece of data as-is, you more often than not don't want to draw
attention to this implementation detail by blowing up its size.

> 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)))))))
Fun fact, you could inline this delete into the "compilerOptions" line
– or sexp at least, by using (= (cute delete '("target" "ES5") <>)
options).  That being said, it's not necessary to do so; the delete you
currently have works fine.

Anyhow, since this isn't a clean alist, I'd use a different variable
name, perhaps "options"?

Cheers
Philip McGrath Nov. 16, 2023, 1:51 a.m. UTC | #6
Hi,

On 11/15/23 20:17, Liliana Marie Prikler wrote:
> Hi,
> 
> Am Mittwoch, dem 15.11.2023 um 19:03 -0500 schrieb 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:
>>>> [...]
>>>>
>>>> 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 was a (_ #t) in the filter, though :)
> In any case, such trivial matches should fit onto one line imho.
> 
>> 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
>> _.
> IMHO, this only makes sense for non-trivial replacements.  If you keep
> some piece of data as-is, you more often than not don't want to draw
> attention to this implementation detail by blowing up its size.
> 

I don't think the content of the right-hand side is relevant: in my 
view, the purpose of the newline is to make the shape of the clause 
clear, especially given that the left-hand side is not an expression. 
The fact that Guix's style forbids square brackets makes the newline 
even more necessary, IMO.

In any case, what arrangement of whitespace in patch-build-script would 
unblock this patch series for you?

>>
>> 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)))))))
> Fun fact, you could inline this delete into the "compilerOptions" line
> – or sexp at least, by using (= (cute delete '("target" "ES5") <>)
> options).  That being said, it's not necessary to do so; the delete you
> currently have works fine.
> 
> Anyhow, since this isn't a clean alist, I'd use a different variable
> name, perhaps "options"?
> 

All of the variables named alist are, in fact, alists.

I noticed, though, that this was inadvertently renaming 
"compilerOptions" to "scripts" and thus effectively patching out all the 
other options, too: things seemed to work regardless, but here's a 
correct version:

>           (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)
>                             `("compilerOptions" @ ,@(delete '("target" . "ES5")
>                                                             alist)))
>                            (other
>                             other))
>                          alist)))))))

Philip
Liliana Marie Prikler Nov. 16, 2023, 7:18 a.m. UTC | #7
Hi,

Am Mittwoch, dem 15.11.2023 um 20:51 -0500 schrieb Philip McGrath:
> Hi,
> 
> On 11/15/23 20:17, Liliana Marie Prikler wrote:
> > Hi,
> > 
> > Am Mittwoch, dem 15.11.2023 um 19:03 -0500 schrieb 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:
> > > > > [...]
> > > > > 
> > > > > 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 was a (_ #t) in the filter, though :)
> > In any case, such trivial matches should fit onto one line imho.
> > 
> > > 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
> > > _.
> > IMHO, this only makes sense for non-trivial replacements.  If you
> > keep
> > some piece of data as-is, you more often than not don't want to
> > draw
> > attention to this implementation detail by blowing up its size.
> > 
> 
> I don't think the content of the right-hand side is relevant: in my 
> view, the purpose of the newline is to make the shape of the clause 
> clear, especially given that the left-hand side is not an expression.
> The fact that Guix's style forbids square brackets makes the newline 
> even more necessary, IMO.
> 
> In any case, what arrangement of whitespace in patch-build-script
> would unblock this patch series for you?
If push comes to shove, I can rearrange the whitespace as I want, given
my committer privileges; it's just that you've complained about me
abusing them before, so I don't really want to.

Anyhow, stuff like (_ #t), (_ #f), (x x), … should all fit onto a
single line.

> > > 
> > > 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)))))))
> > Fun fact, you could inline this delete into the "compilerOptions"
> > line  or sexp at least, by using (= (cute delete '("target" "ES5")
> > <>) options).  That being said, it's not necessary to do so; the
> > delete you currently have works fine.
> > 
> > Anyhow, since this isn't a clean alist, I'd use a different
> > variable name, perhaps "options"?
> 
> All of the variables named alist are, in fact, alists.
TIL.

> I noticed, though, that this was inadvertently renaming 
> "compilerOptions" to "scripts" and thus effectively patching out all
> the other options, too: things seemed to work regardless, but here's
> a correct version:
> 
> >           (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)
> >                             `("compilerOptions" @ ,@(delete
> > '("target" . "ES5")
> >                                                            
> > alist)))
> >                            (other
> >                             other))
> >                          alist)))))))
Now that looks like a proper alist to me :)
diff mbox series

Patch

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 66d09700db..8e22997957 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -64,6 +64,7 @@ 
 ;;; Copyright © 2023 David Thompson <dthompson2@worcester.edu>
 ;;; Copyright © 2023 Christopher Howard <christopher@librehacker.com>
 ;;; Copyright © 2023 Felix Lechner <felix.lechner@lease-up.com>
+;;; Copyright © 2023 Philip McGrath <philip@philipmcgrath.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -98,6 +99,7 @@  (define-module (gnu packages web)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system go)
   #:use-module (guix build-system meson)
+  #:use-module (guix build-system node)
   #:use-module (guix build-system perl)
   #:use-module (guix build-system pyproject)
   #:use-module (guix build-system python)
@@ -168,6 +170,7 @@  (define-module (gnu packages web)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages networking)
   #:use-module (gnu packages node)
+  #:use-module (gnu packages node-xyz)
   #:use-module (gnu packages nss)
   #:use-module (gnu packages openldap)
   #:use-module (gnu packages openstack)
@@ -2343,6 +2346,190 @@  (define-public sassc/libsass-3.5
                                                     "0830pjcvhzxh6yixj82x5k5r1xnadjqzi16kp53213icbly0r9ma")))))))
     (properties '((hidden? . #t)))))
 
+(define-public lessc
+  (package
+    (name "lessc")
+    (version "4.2.0")
+    (source
+     (origin (method git-fetch)
+             (uri (git-reference
+                   (url "https://github.com/less/less.js")
+                   (commit (string-append "v" version))))
+             (sha256
+              (base32 "1b6anlafk7lnayxy3vhsi474jcdah2ffaw2qyac5s2ibxb1wmr54"))
+             (snippet
+              #~(begin
+                  (use-modules (guix build utils))
+                  (delete-file-recursively "packages/less/dist")))
+             (file-name (git-file-name name version))))
+    (build-system node-build-system)
+    (native-inputs (list esbuild))
+    (inputs (list node-copy-anything))
+    (arguments
+     (list
+      #:tests? #f ; many more dependencies
+      #:modules
+      `((guix build node-build-system)
+        (ice-9 match)
+        (guix build utils))
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'chdir
+            (lambda args
+              (chdir "packages/less")))
+          (add-after 'patch-dependencies 'delete-dependencies
+            (lambda args
+              (delete-dependencies
+               '(;; dependencies
+                 "parse-node-version" ; patched out
+                 "tslib" ; probably not needed w/ esbuild
+                 ;; devDependencies
+                 "@less/test-data"
+                 "@less/test-import-module"
+                 "@rollup/plugin-commonjs"
+                 "@rollup/plugin-json"
+                 "@rollup/plugin-node-resolve"
+                 "@typescript-eslint/eslint-plugin"
+                 "@typescript-eslint/parser"
+                 "benny"
+                 "bootstrap-less-port"
+                 "chai"
+                 "cross-env"
+                 "diff"
+                 "eslint"
+                 "fs-extra"
+                 "git-rev"
+                 "globby"
+                 "grunt"
+                 "grunt-cli"
+                 "grunt-contrib-clean"
+                 "grunt-contrib-connect"
+                 "grunt-eslint"
+                 "grunt-saucelabs"
+                 "grunt-shell"
+                 "html-template-tag"
+                 "jit-grunt"
+                 "less-plugin-autoprefix"
+                 "less-plugin-clean-css"
+                 "minimist"
+                 "mocha"
+                 "mocha-headless-chrome"
+                 "mocha-teamcity-reporter"
+                 "nock"
+                 "npm-run-all"
+                 "performance-now"
+                 "phin"
+                 "promise"
+                 "read-glob"
+                 "resolve"
+                 "rollup"
+                 "rollup-plugin-terser"
+                 "rollup-plugin-typescript2"
+                 "semver"
+                 "shx"
+                 "time-grunt"
+                 "ts-node"
+                 "typescript"
+                 "uikit"
+                 ;; optionalDependencies
+                 "errno"
+                 "graceful-fs"
+                 "image-size"
+                 "make-dir"
+                 "mime"
+                 "needle"
+                 "source-map"))))
+          (add-after 'delete-dependencies 'avoid-parse-node-version
+            (lambda args
+              (define version
+                #$(package-version this-package))
+              (substitute* "src/less/index.js"
+                (("import [{] version [}]" orig)
+                 (string-append "// " orig))
+                (("import parseVersion from 'parse-node-version';" orig)
+                 (string-append "// " orig))
+                (("const v = parseVersion[(]`v\\$[{]version[}]`[)];" orig)
+                 (string-append "// " orig))
+                (("(version: )(\\[v\\.major, v\\.minor, v\\.patch],)" _ lhs rhs)
+                 (string-append
+                  lhs
+                  "["
+                  (string-join
+                   (list-head (string-split
+                               version (char-set-complement char-set:digit))
+                              3)
+                   ", ")
+                  "], // "
+                  rhs)))))
+          (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)))))))
+          (add-after 'build 'build-browser
+            (lambda args
+              (invoke "esbuild"
+                      "--bundle"
+                      "--platform=browser"
+                      "--format=cjs"
+                      "--outfile=dist/less.js"
+                      "src/less-browser/bootstrap.js")
+              (invoke "esbuild"
+                      "--bundle"
+                      "--minify"
+                      "--sourcemap"
+                      "--platform=browser"
+                      "--format=cjs"
+                      "--outfile=dist/less.min.js"
+                      "src/less-browser/bootstrap.js"))))))
+    (home-page "https://lesscss.org")
+    (synopsis "Compiler for @acronym{Less} @acronym{CSS} language extension")
+    ;; XXX: @abbr{} seems better for Less (which is always capitalized that
+    ;; way), but it is rejected as invalid Texinfo markup here.
+    (description "@acronym{Less, Leaner Style Sheets} is a
+backwards-compatible language extension for @acronym{CSS}.  This package
+provides @command{lessc}, which compiles Less files to plain @acronym{CSS}.")
+    (license license:asl2.0)))
+
 
 (define-public perl-apache-logformat-compiler
   (package