From: Philip Kaludercic <philipk@posteo.net>
To: Elijah G <eg642616@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: [ELPA] New package: colorful-mode
Date: Mon, 22 Apr 2024 07:51:30 +0000 [thread overview]
Message-ID: <878r15vnr1.fsf@posteo.net> (raw)
In-Reply-To: <CACnP4NLOJBe_j9tNcV8Na-N-sUz1Eh4APfLrg1zo2025MH_M8Q@mail.gmail.com> (Elijah G.'s message of "Fri, 19 Apr 2024 19:07:51 -0600")
[-- Attachment #1: Type: text/plain, Size: 818 bytes --]
Elijah G <eg642616@gmail.com> writes:
> Hello, I want to submit the package colorful-mode to GNU ELPA.
>
> Colorful is a minor mode that allows preview of any color format such
> as color hex and color names in the current buffer in real time with a
> user-friendly approach.
>
> Colorful was based in rainbow-mode source, but colorful diverged in
> other way being more friendly, customizable and allow converting color
> to others with mouse clicks or keybindings such as css rgb/hsl to hex,
> or hex to color name and allow using prefix/suffix as indicator or
> highlight it using overlays, also it has support for mouse clicks for
> changing colors or using keybindings.
>
> The source can be found at: https://github.com/DevelopmentCool2449/colorful-mode
Sounds nice. Here are a few comments and suggestions:
[-- Attachment #2: Type: text/plain, Size: 6755 bytes --]
diff --git a/colorful-mode.el b/colorful-mode.el
index d28db8d..82ddf5e 100644
--- a/colorful-mode.el
+++ b/colorful-mode.el
@@ -30,20 +30,18 @@
;; friendly and effective way based on rainbow-mode.
;;; Code:
+
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Libraries ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; Libraries
(require 'compat)
(require 'color)
(eval-when-compile (require 'subr-x))
+(eval-when-compile (require 'rx))
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Customizable User Options ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; User Options
(defgroup colorful nil
"Preview hex colors values in current buffer.."
@@ -279,9 +277,7 @@ mode is derived from `prog-mode'."
:type '(choice boolean (const :tag "Only in prog-modes" only-prog)))
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Keymaps ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; Keymaps
(defvar-keymap colorful-mode-map
:doc "Keymap when `colorful-mode' is active."
@@ -290,19 +286,14 @@ mode is derived from `prog-mode'."
"C-c c r" #'colorful-convert-and-change-color)
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Internal variables ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; Internal variables
(defvar-local colorful-color-keywords nil
"Font-lock colors keyword to highlight.")
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Internal Functions ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-
-;;;;;;;;;; Base Convertion functions ;;;;;;;;;;
+;;;; Internal Functions
+;;;;; Base Convertion functions
(defun colorful--percentage-to-absolute (percentage)
"Convert PERCENTAGE to a absolute number.
@@ -315,12 +306,13 @@ If PERCENTAGE is above 100%, it's converted to 100."
(string-to-number percentage))))
(defun colorful--latex-rgb-to-hex (rgb)
- "Return LaTex RGB as hexadecimal format. RGB must be a string."
- (let* ((rgb (string-split (string-remove-prefix "{rgb}{" rgb) ","))
- (r (string-to-number (nth 0 rgb)))
- (g (string-to-number (nth 1 rgb)))
- (b (string-to-number (nth 2 rgb))))
- (color-rgb-to-hex r g b)))
+ "Return LaTeX RGB as hexadecimal format. RGB must be a string."
+ ;; just as an alternative idea:
+ (and (string-match "{rgb}{\\([[:digit:]]+\\),\\([[:digit:]]+\\),\\([[:digit:]]+\\)}" rgb)
+ (color-rgb-to-hex
+ (string-to-number (match-string 1 rgb))
+ (string-to-number (match-string 2 rgb))
+ (string-to-number (match-string 3 rgb)))))
(defun colorful--latex-gray-to-hex (gray)
"Return LaTex GRAY as hexadecimal format. GRAY must be a string."
@@ -350,6 +342,8 @@ HSL must be a string."
(string-remove-prefix "hsl(" hsl)
(string-remove-prefix "hsla(" hsl))
(rx (one-or-more (any "," " " "\t" "\n""\r" "\v" "\f")))))
+ ;; what error is being ignored here? if (nth n hsl) is
+ ;; nil, we can check this manually
(h (ignore-errors (/ (string-to-number (nth 0 hsl)) 360.0)))
(s (ignore-errors (/ (string-to-number (nth 1 hsl)) 100.0)))
(l (ignore-errors (/ (string-to-number (nth 2 hsl)) 100.0)))
@@ -367,11 +361,11 @@ HSL must be a string."
"Return color NAME as hex color format.
DIGIT specifies which how much digits per component must have return value."
(if-let* ((color-name (color-name-to-rgb name))
- (color (append color-name `(,digit))))
+ (color (append color-name (list digit))))
(apply #'color-rgb-to-hex color)
(cdr (assoc-string name colorful-html-colors-alist))))
-;;;;;;;;;; User Interactive Functions ;;;;;;;;;;
+;;;;; User Interactive Functions
;;;###autoload
(defun colorful-convert-and-change-color ()
@@ -419,12 +413,12 @@ DIGIT specifies which how much digits per component must have return value."
("Convert and copy color." . copy)))
(result (alist-get
(completing-read prompt choices nil t nil nil)
- choices nil nil 'equal)))
+ choices nil nil #'equal)))
(if (eq result 'copy)
(colorful-convert-and-copy-color)
(colorful-convert-and-change-color))))
-;;;;;;;;;; Coloring functions ;;;;;;;;;;
+;;;;; Coloring functions
(defun colorful--change-color (ov &optional prompt color beg end)
"Return COLOR as other color format.
@@ -582,9 +576,7 @@ converted to a Hex color."
(colorful--colorize-match string beg end)))
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Extra coloring definitions ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; Extra coloring definitions
(defvar colorful-hex-font-lock-keywords
`((,(rx (seq (not (any "&"))
@@ -639,6 +631,7 @@ converted to a Hex color."
"Function for add hex colors to `colorful-color-keywords'.
This is intended to be used with `colorful-extra-color-keyword-functions'."
(dolist (colors colorful-hex-font-lock-keywords)
+ ;; why are you using `add-to-list' here?
(add-to-list 'colorful-color-keywords colors t)))
(defvar colorful-color-name-font-lock-keywords
@@ -730,9 +723,7 @@ This is intended to be used with `colorful-extra-color-keyword-functions'."
(add-to-list 'colorful-color-keywords colors t)))
\f
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; Minor mode defintinions ;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;; Minor mode defintinions
(defun colorful--turn-on ()
"Helper function for turn on `colorful-mode'."
@@ -759,7 +750,7 @@ This is intended to be used with `colorful-extra-color-keyword-functions'."
;;;###autoload
(define-minor-mode colorful-mode
"Preview any color in your buffer such as hex, color names, CSS rgb in real time."
- :lighter nil :keymap colorful-mode-map
+ :global nil
(if colorful-mode
(colorful--turn-on)
(colorful--turn-off))
[-- Attachment #3: Type: text/plain, Size: 205 bytes --]
Regarding the headers, my changes would make it compatible with
outline-minor-mode for Elisp, but if you prefer your style, then you can
also adjust `outline-regexp'.
--
Philip Kaludercic on peregrine
next prev parent reply other threads:[~2024-04-22 7:51 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 1:07 [ELPA] New package: colorful-mode Elijah G
2024-04-22 7:51 ` Philip Kaludercic [this message]
2024-04-23 2:49 ` Elijah G
2024-04-27 10:12 ` Philip Kaludercic
2024-04-27 16:44 ` Elijah G
2024-04-27 21:12 ` Philip Kaludercic
2024-04-28 23:39 ` Elijah G
2024-04-29 5:47 ` Philip Kaludercic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878r15vnr1.fsf@posteo.net \
--to=philipk@posteo.net \
--cc=eg642616@gmail.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).