mbox series

[bug#68935,v3,0/7] Add insert option to guix import.

Message ID cover.1708461547.git.herman@rimm.ee
Headers show
Series Add insert option to guix import. | expand

Message

Herman Rimm Feb. 20, 2024, 8:45 p.m. UTC 
Hi,

Thanks for the feedback. Biggest issue I see is, e.g.:

  (define-public package-bar)
  ;; Unlikely comment about package-bar.
  
  (define for-package-foo)
  ;; Comment related to package-foo.
  (define-public package-foo)

after inserting package-baz becomes:

  (define-public package-bar)
  ;; Unlikely comment about package-bar.
  
  (define for-package-foo)
  ;; Comment related to package-foo.
+ (define-public package-baz)
+
  (define-public package-foo)

but for a language read from top-to-bottom I would rather want:

  (define-public package-bar)
+  
+ (define for-package-baz)
  ;; Unlikely comment about package-bar.

  (define for-package-foo)
  ;; Comment related to package-foo.
  (define-public package-foo)

This would be the case if package-baz is inserted after package-bar,
rather than before package-foo. I don't intend to implement this though,
and for the large gnu/packages/crates-*.scm files, where alphabetical
--insert is most useful, I believe this issue is the least likely to
occur. So despite the issue, I think these patches can be upstreamed.

Cheers,
Herman

Herman Rimm (7):
  doc: Note SVN dependency of texlive importer.
  import: Wrap package expressions with define-public.
  utils: Add insert-expression procedure.
  utils: Add find-definition-insertion-location procedure.
  import: Insert packages into modules alphabetically.
  import: Discard args after --version and --help.
  import: Do not return package name with json importer.

 doc/guix.texi           | 22 ++++++++---
 doc/package-hello.json  |  6 +--
 guix/import/json.scm    | 13 +++----
 guix/scripts/import.scm | 82 ++++++++++++++++++++++++++++-------------
 guix/utils.scm          | 30 +++++++++++++++
 tests/utils.scm         | 28 ++++++++++++++
 6 files changed, 140 insertions(+), 41 deletions(-)


base-commit: e3c612a7de679c96b9eafdb0da500dcc18d9a101

Comments

Ludovic Courtès Feb. 23, 2024, 5:53 p.m. UTC | #1 
Hi,

Herman Rimm <herman@rimm.ee> skribis:

> after inserting package-baz becomes:
>
>   (define-public package-bar)
>   ;; Unlikely comment about package-bar.
>   
>   (define for-package-foo)
>   ;; Comment related to package-foo.
> + (define-public package-baz)
> +
>   (define-public package-foo)

I think that’s an acceptable limitation.  Also, usually one would write:

  (define-public foo
    ;; Comment related to foo.
    (package …))

This case is correctly handled.

Ludo’.
Ludovic Courtès Feb. 23, 2024, 7:52 p.m. UTC | #2 
Hi!

Herman Rimm <herman@rimm.ee> skribis:

>   doc: Note SVN dependency of texlive importer.
>   import: Wrap package expressions with define-public.
>   utils: Add insert-expression procedure.
>   utils: Add find-definition-insertion-location procedure.
>   import: Insert packages into modules alphabetically.
>   import: Discard args after --version and --help.
>   import: Do not return package name with json importer.

Pushed as b386c11e7804e0b577411d930b60f1e0a4a0382c, thanks!

Ludo’.