On Tue, Dec 07, 2021 at 02:00:02AM +0700, Blake Shaw via Guix-patches via wrote:
> +(define-module (gnu packages notcurses)

Thank you for this patch!

As part of the review process, I made some changes locally, as shown in
the diff below. I've also attached the complete revised patch, which
includes indentation changes that are not shown in the diff. I did not
include the indentation changes in the diff, so that it would be easier
to show what has changed. I think the attached patch is more or less
ready to push. Let me know what you think.

------
diff --git a/gnu/packages/notcurses.scm b/gnu/packages/notcurses.scm
index 00e2fd92db..1fc176c27c 100644
--- a/gnu/packages/notcurses.scm
+++ b/gnu/packages/notcurses.scm
@@ -23,13 +23,13 @@ (define-module (gnu packages notcurses)
   #:use-module (guix git-download)
   #:use-module (guix build-system cmake)
   #:use-module ((guix licenses) #:prefix license:)
-  #:use-module (gnu packages gcc)
+  #:use-module (gnu packages check)
+  #:use-module (gnu packages haskell-xyz)
   #:use-module (gnu packages video)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages compression)
-  #:use-module (gnu packages libunistring)
-  #:use-module (ice-9 match))
+  #:use-module (gnu packages libunistring))
 
 (define-public notcurses
   (package
@@ -46,28 +46,33 @@ (define-public notcurses
           (base32 "1y9s77m1pp6syfml559d8dvif61y6zjldrdx1zri18q9sr0zqm9m"))))
       (build-system cmake-build-system)
       (arguments
-       `(#:tests? #f
-                  #:make-flags
-                  (list ,(string-append "CC=" (cc-for-target))
-                   (string-append "prefix="))
-                  #:configure-flags
-                  (map (lambda (s)
-                         (string-append "-D" s))
-                       '("USE_CPP=off"     "USE_COVERAGE=off" ;;here we set the default cmake
-                         "USE_DOXYGEN=off" "USE_DOCTEST=off"  ;;configuration. FSG=FSF approved only
-                         "USE_GPM=off"     "USE_MULTIMEDIA=ffmpeg" ;;other choices based on 
-                         "USE_PANDOC=off"  "FSG_BUILD=ON"))))      ;;reducing package footprint
+       `(#:make-flags
+         (list (string-append "CC=" ,(cc-for-target)))
+         ;; These flags are documented in 'INSTALL.md' in the source distribution.
+         #:configure-flags
+         '(;; Do not build "coverage"
+           "-DUSE_COVERAGE=off"
+           ;; Do not build HTML documentation
+           "-DUSE_DOXYGEN=off"
+           ;; Don't include mouse support
+           "-DUSE_GPM=off"
+           ;; Use FFmpeg for multimedia support
+           "-DUSE_MULTIMEDIA=ffmpeg"
+           ;; Follow the Debian Free Software Guidelines
+           ;; Yes, 'INSTALL.md' says that "OFF" means to omit non-free code.
+           "-DFSG_BUILD=OFF")))
       (native-inputs
-       `(("gcc-toolchain" ,gcc)
-         ("ncurses" ,ncurses)
-         ("pkg-config" ,pkg-config)))
+       `(("pkg-config" ,pkg-config)
+         ("pandoc" ,pandoc)
+         ("doctest" ,doctest)))
       (inputs
        `(("ffmpeg" ,ffmpeg)
          ("libdeflate" ,libdeflate)
          ("libunistring" ,libunistring)
+         ("ncurses" ,ncurses)
          ("zlib" ,zlib)))
-      (synopsis "Library facilitating complex textual user interfaces on modern terminals")
-      (description "Notcurses is a library for building complex 
-textual user interfaces on modern terminals. It does not use ncurses, while it does make use of libtinfo from that package.")
-      (home-page "https://notcurses.com/html/")
+      (synopsis "Textual user interfaces")
+      (description "Notcurses is a library for building complex textual user
+interfaces on modern terminals.")
+      (home-page "https://notcurses.com")
       (license license:asl2.0)))
------

Concretely, here are the changes that I made:

I enabled the tests. We always run upstream test suites in Guix
packages, unless there is a reason not to. This helps us validate that
the package works properly, which helps us and upstream. This also means
that I added a dependency on doctest.

I removed the "prefix=" make flag. The package was installed correctly
without it, so this change seems okay.

I enabled building with C++. According to README.md, this implementation
is more fully-featured, and in Guix we aim to provide fully-featured
packages.

I build the man pages with Pandoc. Guix packages should include
documentation when it is available. This makes the package depend on
Pandoc.

Regarding DFSG_BUILD, I set it to "OFF". The notcurses INSTALL.md says
this: "DFSG_BUILD=off: leave out all content considered non-free under
the Debian Free Software Guidelines". That means that we want this to be
OFF in order to follow the Debian guidelines. [0] Or is that incorrect?

I also removed GCC from native-inputs. GCC is available "by default"
with cmake-build-system.

I moved the ncurses dependency to "inputs", because the built
package does keep a run-time reference to it. You can consider
native-inputs as things that only need to be used while building [1].

Based on feedback from `guix lint notcurses`, I edited the synopsis and
description. Maybe the description could be longer.

Please let me know what you think about the revised patch, and thanks
again for contributing!

[0] By the way, the DFSG is not equivalent to the FSDG, which is from
the FSF and which we follow. They are certainly similar, but they are
not related.

[1] https://guix.gnu.org/manual/en/html_node/package-Reference.html