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.