Subject: Re: [bug#53648] [PATCH] gnu: fcitx5-gtk: Add GTK 4 support.

From: Maxime Devos <[email-masked]>
Date: Tue, 08 Feb 2022 11:02:10 +0100

Zhu Zihao schreef op di 08-02-2022 om 12:19 [+0800]:
>        (modify-phases %standard-phases
>           (add-before 'configure 'patch-install-prefix
>             (lambda* (#:key outputs #:allow-other-keys)
> -             (let ((out (assoc-ref outputs "out"))
> -                   (gtk2 (assoc-ref outputs "gtk2"))
> -                   (gtk4 (assoc-ref outputs "gtk4")))

Perhaps I should have mentioned this earlier, but this can be
simplified a little with G-exps:

   (out #$output)
   (gtk2 #output:gtk2)
   (gtk4 #$output:gtk4)

> -               ;; Install GTK+ 2 input method module to its own
> output.
> -               (substitute* "gtk2/CMakeLists.txt"
> +             ;; Take care of different versions of GTK because this
> package
> +             ;; provides IM module for GTK application to use input
> method.
> +             (define (split-immodule gtk-version)

gtk-version -> gtk-output

> +               (substitute* (string-append gtk-version
> "/CMakeLists.txt")
>                   (("\\$\\{CMAKE_INSTALL_LIBDIR\\}")
> -                  (string-append gtk2 "/lib")))
> +                  (string-append (assoc-ref outputs gtk-version)
> "/lib"))))

(assoc-ref outputs gtk-version) -> gtk-output

>  
> -               ;; Install for GTK 4.
> -               (substitute* "gtk4/CMakeLists.txt"
> -                 (("\\$\\{CMAKE_INSTALL_LIBDIR\\}")
> -                  (string-append gtk4 "/lib")))))))))
> +             (for-each split-immodule '("gtk2" "gtk3" "gtk4")))))))

'("gtk2" "gtk3" "gtk4") --> (list gtk2 gtk3 gtk4)

YMMV whether this change would be an improvement.

Likewise, %output can be eliminated by using #$output..

>      (inputs
>       `(("fcitx5" ,fcitx5)
>         ("fmt" ,fmt)
> @@ -232,12 +228,22 @@ (define-public fcitx5-gtk
>      (native-inputs
>       (list extra-cmake-modules pkg-config
>             `(,glib "bin")))           ;for glib-genmarshal
> -    ;; TODO: Add "lib" output to reduce the closure size of "gtk2".
> -    (outputs '("out" "gtk2" "gtk4"))
> +    (outputs '("out" "gtk2" "gtk3" "gtk4"))
>      (home-page "https://github.com/fcitx/fcitx5-gtk")
>      (synopsis "Glib based D-Bus client and GTK IM module for Fcitx
> 5")
> -    (description "Fcitx5-gtk provides a Glib based D-Bus client and
> IM module
> -for GTK+2/GTK+3 application.")
> +    (description "Fcitx5-gtk provides following functionality in
> corresponding
> +output:
> +
> +@table @code
> +@item out
> +GLib based D-Bus client of Fcitx5.
> +@item gtk2
> +IM module for GTK+2 applications.
> +@item gtk3
> +IM module for GTK+3 applications.
> +@item gtk4
> +IM module for GTK4 applications.
> +@end table")
>      (license license:lgpl2.1+)))

Looks nice and orderrly.
A few definite articles would seem to be in place though:
‘Fcitx5-gtk provides the following functionality in the corresponding
output:’

I'm not 100% sure but I think it's ‘GLib-based’ (with a hyphen) instead
of ’GLib based’.

Greetings,
Maxime.