[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu-maintenance: update-package-source: Only update the desi
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] gnu-maintenance: update-package-source: Only update the desired package. |
Date: |
Tue, 05 Apr 2016 11:45:56 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
宋文武 <address@hidden> skribis:
> Fixes <http://bugs.gnu.org/22693>.
> Suggested by Andy Wingo.
>
> * guix/upstream.scm (update-package-source): Use a customized 'substitute'
> to work within lines of the package's source.
[...]
> + (define (substitute+ file start end pattern+procs)
> + ;; Same as substitute, but within lines from START to END.
Please make it a top-level procedure, for clarity.
> + (define (package-location-line-start package)
> + (location-line (package-location package)))
> +
> + (define (package-location-line-end package)
> + (define (goto port line column)
> + (unless (and (= (port-column port)) (- column 1)
> + (= (port-line port) (- line 1)))
> + (unless (eof-object? (read-char port))
> + (goto port line column))))
> +
> + (match (package-location package)
> + (($ <location> file line column)
> + (call-with-input-file (search-path %load-path file)
> + (lambda (port)
> + (goto port line column)
> + (match (read port)
> + (('package _ ...)
> + (1+ (port-line port)))))))))
I think you should add a ‘match’ case here, for when (read port) returns
something that is not a ‘package’ form (that could happen.)
The docstring should specify that this can return #f, and callers should
fall back to a wild guess, like:
(or (package-location-end-line p)
(+ (package-location-start-line p) 30))
But overall the approach looks good to me!
Could you:
1. Call these two procedure ‘package-location-start-line’ and
‘package-location-end-line’ (I think it sounds better from a
grammatical standpoint ;-))?
2. Move them to (guix packages), next to ‘package-field-location’?
3. Add tests in tests/packages.scm?
The (guix upstream) changes would come in a separate patch.
WDYT?
Thank you!
Ludo’.
Re: [PATCH] gnu-maintenance: update-package-source: Only update the desired package.,
Ludovic Courtès <=