[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#28185: [PATCH] build: emacs-build-system: Make the install phase mor
bug#28185: [PATCH] build: emacs-build-system: Make the install phase more helpful.
Fri, 1 Sep 2017 22:08:55 +0100
On Fri, 01 Sep 2017 10:32:18 +0530
Arun Isaac <address@hidden> wrote:
> >> > + (if (null? files-to-install)
> >> > + (begin
> >> > + (format #t "error: No files found to install.\n")
> >> > + (find-files source (lambda (file stat)
> >> > + (install-file? file stat
> >> > #:verbose? #t)))
> >> I understand that the verbose output also shows the regexp due to
> >> which the file was excluded, and that has debugging value. But,
> >> perhaps, it'll be better to just print that here instead of a call
> >> back to `install-file?'.
> >> WDYT?
> > I put this in install-file? as I didn't want to duplicate the
> > behaviour inside the function, firstly to avoid duplication, but
> > also to avoid the possiblily that if they were duplicated, they
> > would diverge in behaviour. I'm happy to experiment with splitting
> > the "verbose" output out of install-file though?
> Yes, I meant that you should split the verbose output out of
> `install-file?'. But, it's not a big deal. I'm only nitpicking. Do go
> ahead with your original code if you think it's alright.
> >> > + #f)
> >> > + (begin
> >> > + (for-each
> >> > + (lambda (file)
> >> > + (let* ((stripped-file (string-drop file
> >> > (string-length source)))
> >> > + (target-file (string-append target-directory
> >> > stripped-file)))
> >> > + (format #t "`~a' -> `~a'~%" file target-file)
> >> > + (install-file file (dirname target-file))))
> >> > + files-to-install)
> >> > + #t))))
> >> Could you please rewrite this section using `cond' instead of `if'?
> >> That way, we can get rid of the `begin'. Also, it might be better
> >> if we used "positive logic" -- meaning, we first check for
> >> truthiness of `files-to-install' instead of checking for (null?
> >> files-to-install). Then, the else statement would take care of the
> >> empty files-to-install case, and we would never have to call
> >> `null?' explicitly.
> > I tried this, but it seems that cond will treat '() as if it
> > evaluates to true:
> > scheme@(guile-user)> (cond ('() #t) (else #f))
> > $1 = #t
> > So this might need to still use null?, or even (not (null?
> > files-to-install))?
> Ah, yes. Sorry, I'm confusing scheme's behaviour with that of other
> lisps. We still need `null?'.
> The patch LGTM.
Great, I've pushed this now :)
Description: OpenPGP digital signature
|[Prev in Thread]
||[Next in Thread]|
- bug#28185: [PATCH] build: emacs-build-system: Make the install phase more helpful.,
Christopher Baines <=