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