Re: Submission of modus-vague package to NonGNU ELPA
Philip Kaludercic <[email protected]>
| Newsgroups | gmane.emacs.devel |
|---|---|
| Message-ID | <[email protected]> |
Panigrahi Ashish <[email protected]> writes: >> [... 27 lines elided] >> >> No problem! See https://elpa.gnu.org/contributing.html, we prefer >> package submissions to GNU ELPA by default, unless the maintainer has a >> strong preference against it or is not in a position to sign the >> copyright assignment. If you have any more questions about this or >> anything else relating to ELPA, don't hesitate to ask us :) > > Sounds good! Thanks. > >>> If I understand correctly, there would be paperwork involved for copyright assignment to the FSF. >> >> Yes, I have CC'ed Eli who can send you the copyright form. > > At this point I'm happy to receive feedback on the package, to make it > ready for submitting to GNU ELPA once the copyright form is signed. OK, first point, I'd add an .elpaignore file to exclude the screenshots directory. I have taken a look at the source code and have the following comments and suggestions: I suppose Prot will be able to answer some of the questions as well. The main point is that you will have to adjust the copyright header in both files to attribute the copyright to the FSF. Currently only one file does so, which is peculiar. > Thanks. > > Kind regards, > Ashish
(unnamed)
(text/x-patch, 3.6 KB)
diff --git a/modus-vague-theme.el b/modus-vague-theme.el
index 3f903b1..e62d9b3 100644
--- a/modus-vague-theme.el
+++ b/modus-vague-theme.el
@@ -125,7 +125,7 @@
(border-mode-line-inactive unspecified))))
(defconst modus-vague-custom-faces
- '(
+ '( ;are you sure you want to quote underneath a backquote?
`(git-commit-summary ((,c :inherit bold :foreground ,iris))))
"Custom faces overriding the default faces of Modus themes.")
diff --git a/modus-vague.el b/modus-vague.el
index ab41fc7..0b31ca6 100644
--- a/modus-vague.el
+++ b/modus-vague.el
@@ -1,8 +1,7 @@
;;; modus-vague.el --- Emacs port of the popular vague theme -*- lexical-binding:t -*-
-;; Copyright (c) 2026 Ashish Panigrahi
+;; Copyright (c) 2026 Free Software Foundation, Inc.
-;; Title: Modus Vague
;; Author: Ashish Panigrahi <[email protected]>
;; Maintainer: Ashish Panigrahi <[email protected]>
;; URL: https://github.com/paniash/modus-vague
@@ -26,6 +25,7 @@
;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
;;; Commentary:
+
;; This is the Emacs port of the popular vague theme from neovim.
;; It is built on top of Modus themes, thus providing excellent
;; support for most major and minor modes.
@@ -39,14 +39,20 @@
;;;; Basics for building on top of Modus
(defgroup modus-vague ()
- "The vague theme ported to Emacs. A cool, dark, low contrast colorscheme.
-Pastel yet vivid, like a fleeting memory..."
+ "The vague theme ported to Emacs.
+A cool, dark, low contrast colorscheme. Pastel yet vivid, like a
+fleeting memory..."
:group 'faces
:group 'modus-themes
:link '(info-link "(modus-themes) Top")
+ :link '(url-link "https://github.com/vague-theme/vague
+")
:prefix "modus-vague-"
:tag "Modus Vague Theme")
+;; is this `defconst' dance here necessary, as in that Modus themes
+;; expects these symbols to be defined, or couldn't you inline it?
+
(defconst modus-vague-themes
'(modus-vague)
"List of symbols with the Modus vague theme.")
@@ -68,6 +74,11 @@ Pastel yet vivid, like a fleeting memory..."
"The Modus vague theme."
dark modus-themes-vivendi-palette modus-vague-palette modus-vague-palette-overrides)))
+;; the function below appears to be idempotent, so it seems like you
+;; don't need to prevent it from being re-loaded (which is usually not
+;; an issue if people use `require'). Also it seems like you could
+;; just write the `modus-themes-declare' and `modus-themes-register'
+;; on the top level.
(defvar modus-vague--declared-p nil)
(defun modus-vague-declare-themes ()
@@ -84,12 +95,11 @@ Pastel yet vivid, like a fleeting memory..."
;;;###autoload
(define-minor-mode modus-vague-take-over-modus-themes-mode
- "When enabled, all Modus themes commands consider only Modus vague
-theme. Alternatively, use the commands `modus-vague-list-colors',
-`modus-vague-list-colors-current'. They are all designed to only
+ "When enabled, Modus themes commands consider only Modus vague theme.
+Alternatively, use the commands `modus-vague-list-colors',
+`modus-vague-list-colors-current'. They are all designed to only
consider the Modus vague theme."
- :global t
- :init-value nil)
+ :global t)
(cl-defmethod modus-themes-get-themes (&context (modus-vague-take-over-modus-themes-mode (eql t)))
"Return list of Modus vague theme, per `MODUS-VAGUE-TAKE-OVER-MODUS-THEMES-MODE'."
@@ -101,6 +111,8 @@ consider the Modus vague theme."
;;;; Convenience commands
+;; It would be nice if Mo
+
;;;###autoload (autoload 'modus-vague-list-colors "modus-vague")
(modus-themes-define-derivative-command modus-vague list-colors)