guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#47852] [PATCH] gnu: Add sc-im


From: Jack Hill
Subject: [bug#47852] [PATCH] gnu: Add sc-im
Date: Sat, 17 Apr 2021 19:29:22 -0400 (EDT)
User-agent: Alpine 2.21 (DEB 202 2017-01-01)

On Sat, 17 Apr 2021, jgart via Guix-patches via wrote:

Hi Guix!

Attached is a patch for sc-im, a terminal based spreadsheet program providing a 
vim-like experience.

Interesting program, thanks for working on the package

I added Ekaitz as a co-author and added both of our copyrights to the top of 
the file.

Glad to see this work being picket up and that we can collaborate across time.

This new version of sc-im is from 16 days ago.

I'm currently getting the following two linter warnings:

the source file name should contain the package name
permanent redirect from https://github.com/andmarti1424/sc-im.git to 
https://github.com/andmarti1424/sc-im

Any suggestions for what I need to adjust to make those pass?

I have some inline suggestions as follows:


From 62b2b692329f8db791db08700821111238ed40be Mon Sep 17 00:00:00 2001
From: jgart <jgart@dismail.de>
Date: Sat, 17 Apr 2021 17:43:16 -0400
Subject: [PATCH] gnu: Add sc-im.

    * gnu/packages/visidata.scm: New file.

Should be sc-im.scm there :)

I have no opinion on if this warrants its own file, but I expect others would be willing to comment.

    * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.

    Co-authored-by: Ekaitz Zarraga <ekaitz@elenq.tech>


+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2020 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2021 jgart <jgart@dismail.de>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages sc-im)
+  #:use-module (guix packages)
+  #:use-module (guix git-download)
+  #:use-module (guix download)
+  #:use-module (guix build-system gnu)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages maths)
+  #:use-module (gnu packages statistics)
+  #:use-module (gnu packages xml)
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages ncurses))
+
+(define-public sc-im
+  (let ((commit-ref "75ae3806844821cba1b2e3fdb9237d737944e850"))
+    (package
+      (name "sc-im")
+      (version "0.8.1")
+      (source (origin
+                (method git-fetch)
+                (uri
+                  (git-reference
+                    (url "https://github.com/andmarti1424/sc-im.git";)
+                    (commit commit-ref)))

You can remove the .git from the url to clear up the permanent redirect lint warning.

In other packages, we use the tag directly with something like

(commit (string-append "v" version))

and are thus able to forgo the let binding. Unless there is a reason to think upstream will move the tag to a different commit, I recommend doing that here too.

Also, adding `(file-name (git-file-name name version))` to the origin specification will clear up the file name lint warning.

+                (sha256
+                  (base32
+                    "1i1yq5mh9d7yi1bkgaq4p1lr8zrxhlvqmjnj33wmg5v6vpfim1h0"))))
+      (build-system gnu-build-system)
+      (arguments
+        ;; There are no tests at the moment.
+        ;; https://github.com/andmarti1424/sc-im/issues/537
+        ;; https://github.com/andmarti1424/sc-im/pull/385
+        `(#:tests? #f

Thanks for adding the comment about the lack of test.

+          #:make-flags (list "-C" "src" "CC=gcc"

We prefer `(string-append "CC=" ,(cc-for-target))` which helps when cross-compiling

+                         (string-append "prefix=" (assoc-ref %outputs "out")))
+          #:phases
+            (modify-phases
+               %standard-phases
+                 (delete 'configure))))
+      (inputs
+        `(("gnuplot" ,gnuplot)
+          ("libxls" ,libxls)
+          ("libxlsxwriter" ,libxlsxwriter)
+          ("libxml2" ,libxml2)
+          ("libzip" ,libzip)
+          ("ncurses" ,ncurses)))
+      (native-inputs
+        `(("pkg-config" ,pkg-config)
+          ("which" ,which)
+          ("bison" ,bison)))
+      (synopsis "Spreadsheet program with vim-like keybindings")
+      (description
+ "@code{sc-im} is a highly configurable spreadsheet program
+ providing a vim-like experience.  @code{sc-im} supports @{gnuplot} 
interaction,
+ functions for sorting and filtering, 256 color support, and much more.")
+      (home-page "https://github.com/andmarti1424/sc-im";)
+      (license license:bsd-4))))
--
2.29.3


Can you send an updated patch?

Best,
Jack

reply via email to

[Prev in Thread] Current Thread [Next in Thread]