[ELPA] New package: colorful-mode

[ELPA] New package: colorful-mode

[Elijah G]

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

Thanks.

Re: [ELPA] New package: colorful-mode

[Philip Kaludercic]

[-- 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

Re: [ELPA] New package: colorful-mode

[Elijah G]

Hi Philip, thank you for your suggestions, I've now added
some of them to my package, here are my answers to your questions.

On Mon, Apr 22, 2024 at 1:51 AM Philip Kaludercic <philipk@posteo.net> wrote:
>
>  (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

I had to add ignore-errors because it nth was throwing an error when
nth is nil, it was bugging the other overlays in the next lines, using
ignore-errors allows returning nil in case it can't be done and break the
if-let conditional without affecting the other overlays.

>              (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)))

>  (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)))

It was the way I found for adding the other color keywords,
i found that using add-to-list with a buffer-local variable doesn't
override the other buffer ones, If you know another way in how
I would gladly implement it in my package.
Thanks.

Re: [ELPA] New package: colorful-mode

[Philip Kaludercic]

Elijah G <eg642616@gmail.com> writes:

> Hi Philip, thank you for your suggestions, I've now added
> some of them to my package, here are my answers to your questions.
>
> On Mon, Apr 22, 2024 at 1:51 AM Philip Kaludercic <philipk@posteo.net> wrote:
>>
>>  (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
>
> I had to add ignore-errors because it nth was throwing an error when
> nth is nil, it was bugging the other overlays in the next lines, using
> ignore-errors allows returning nil in case it can't be done and break the
> if-let conditional without affecting the other overlays.

What I am trying to say, is that ignore-errors is a rather coarse tool.
It will swallow and forget all errors that might arise, which can make
debugging difficult if something else goes wrong.  My advice is to use
something like

  (and (nth 0 hsl) (/ (string-to-number (nth 0 hsl)) 360.0))

unless I am misunderstanding your response somehow.

>>              (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)))
>
>>  (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)))
>
> It was the way I found for adding the other color keywords,
> i found that using add-to-list with a buffer-local variable doesn't
> override the other buffer ones, If you know another way in how
> I would gladly implement it in my package.

I would have just used `push', as the `add-to-list' docstring advises.

> Thanks.

-- 
	Philip Kaludercic on peregrine

Re: [ELPA] New package: colorful-mode

[Elijah G]

On Sat, Apr 27, 2024 at 4:12 AM Philip Kaludercic <philipk@posteo.net> wrote:
>
> Elijah G <eg642616@gmail.com> writes:
>
> > Hi Philip, thank you for your suggestions, I've now added
> > some of them to my package, here are my answers to your questions.
> >
> > On Mon, Apr 22, 2024 at 1:51 AM Philip Kaludercic <philipk@posteo.net> wrote:
> >>
> >>  (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
> >
> > I had to add ignore-errors because it nth was throwing an error when
> > nth is nil, it was bugging the other overlays in the next lines, using
> > ignore-errors allows returning nil in case it can't be done and break the
> > if-let conditional without affecting the other overlays.
>
> What I am trying to say, is that ignore-errors is a rather coarse tool.
> It will swallow and forget all errors that might arise, which can make
> debugging difficult if something else goes wrong.  My advice is to use
> something like
>
>   (and (nth 0 hsl) (/ (string-to-number (nth 0 hsl)) 360.0))
>
> unless I am misunderstanding your response somehow.
>
> >>              (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)))
> >
> >>  (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)))
> >
> > It was the way I found for adding the other color keywords,
> > i found that using add-to-list with a buffer-local variable doesn't
> > override the other buffer ones, If you know another way in how
> > I would gladly implement it in my package.
>
> I would have just used `push', as the `add-to-list' docstring advises.
>
> > Thanks.
>
> --
>         Philip Kaludercic on peregrine

Thank you, I've now updated the package to include the advices.
Now I think it's ready to submit it to ELPA.

Re: [ELPA] New package: colorful-mode

[Philip Kaludercic]

Elijah G <eg642616@gmail.com> writes:

> Thank you, I've now updated the package to include the advices.
> Now I think it's ready to submit it to ELPA.

Looks like it, I've added it to elpa.git and it should appear within the
next few hours.  Thank you for your contribution!

-- 
	Philip Kaludercic on icterid

Re: [ELPA] New package: colorful-mode

[Elijah G]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

El sáb., 27 de abril de 2024 3:12 p. m., Philip Kaludercic <
philipk@posteo.net> escribió:

> Elijah G <eg642616@gmail.com> writes:
>
> > Thank you, I've now updated the package to include the advices.
> > Now I think it's ready to submit it to ELPA.
>
> Looks like it, I've added it to elpa.git and it should appear within the
> next few hours.  Thank you for your contribution!
>
> --
>         Philip Kaludercic on icterid
>

Thank you, I have a question, how can I commit the changes I do from my
package to ELPA?

[-- Attachment #2: Type: text/html, Size: 1162 bytes --]

Re: [ELPA] New package: colorful-mode

[Philip Kaludercic]

Elijah G <eg642616@gmail.com> writes:

> El sáb., 27 de abril de 2024 3:12 p. m., Philip Kaludercic <
> philipk@posteo.net> escribió:
>
>> Elijah G <eg642616@gmail.com> writes:
>>
>> > Thank you, I've now updated the package to include the advices.
>> > Now I think it's ready to submit it to ELPA.
>>
>> Looks like it, I've added it to elpa.git and it should appear within the
>> next few hours.  Thank you for your contribution!
>>
>> --
>>         Philip Kaludercic on icterid
>>
>
> Thank you, I have a question, how can I commit the changes I do from my
> package to ELPA?

The elpa.git repository regularly synchronises commits from your
upstream repository.  So you don't have to do anything manually.  If you
want the ELPA build server to release a new version of your  package,
all you have to do is bump the ;; Version: ... line in the main file and
that will trigger a release based on exactly that commit.

-- 
	Philip Kaludercic on peregrine