[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#67019] [PATCH 03/16] gnu: Add lessc.
From: |
Philip McGrath |
Subject: |
[bug#67019] [PATCH 03/16] gnu: Add lessc. |
Date: |
Wed, 15 Nov 2023 14:35:31 -0500 |
User-agent: |
Mozilla Thunderbird |
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
- [bug#67019] [PATCH 00/16] gnu: Add KaTeX, lessc, and flow-remove-types., Philip McGrath, 2023/11/09
- [bug#67019] [PATCH 01/16] gnu: Add node-is-what., Philip McGrath, 2023/11/09
- [bug#67019] [PATCH 02/16] gnu: Add node-copy-anything., Philip McGrath, 2023/11/09
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Philip McGrath, 2023/11/09
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Liliana Marie Prikler, 2023/11/10
- [bug#67019] [PATCH 03/16] gnu: Add lessc.,
Philip McGrath <=
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Liliana Marie Prikler, 2023/11/15
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Philip McGrath, 2023/11/15
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Liliana Marie Prikler, 2023/11/15
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Philip McGrath, 2023/11/15
- [bug#67019] [PATCH 03/16] gnu: Add lessc., Liliana Marie Prikler, 2023/11/16
- [bug#67019] [PATCH v2 00/16] gnu: Add KaTeX, lessc, and flow-remove-types., Philip McGrath, 2023/11/16
- [bug#67019] [PATCH v2 02/16] gnu: Add node-copy-anything., Philip McGrath, 2023/11/16
- [bug#67019] [PATCH v2 03/16] gnu: Add lessc., Philip McGrath, 2023/11/16
- [bug#67019] [PATCH v2 01/16] gnu: Add node-is-what., Philip McGrath, 2023/11/16
- [bug#67019] [PATCH v2 11/16] gnu: Add flow-remove-types., Philip McGrath, 2023/11/16