Re: [PATCH] Flymake Support Indicator Errors in Margin

[Philip Kaludercic <philipk@posteo.net>]

Elijah G <eg642616@gmail.com> writes:

> Hello developers,
> this is my first time contributing to emacs,
> i made support for displaying errors in margin for flymake like flycheck
> does,
> This allows indicating errors in both text and graphical mode.
>
> I've sent my copyright assignment request.
>
> Since this is my first time contributing i would like to hear your
> suggestions and thoughts
>
> Thanks.
> From 0d4539c6b8483eb13cfcdde3d73c71e430fdb30b Mon Sep 17 00:00:00 2001
> From: Elias G <eg642616@gmail.com>
> Date: Mon, 11 Mar 2024 13:35:06 -0600
> Subject: [PATCH] Flymake Support Indicator Errors in Margin
>
> ---
>  etc/NEWS                  |  11 ++++
>  lisp/progmodes/flymake.el | 117 +++++++++++++++++++++++++++++++++-----
>  2 files changed, 114 insertions(+), 14 deletions(-)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 19cd170e5c7..83cf1ba9962 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1121,6 +1121,17 @@ in a clean environment.
>  
>  ** Flymake
>  
> ++++
> +*** New user option 'flymake-indicator-type'
> +Indicate which indicator type to use for display errors.
> +
> +The value can be nil (dont indicate errors but just highlight them),
                            ^
                            don't
                            
> +fringes (use fringes) or margins (use margins).

But I am not sure if you really need to document this in NEWS.  In fact,
it might be possible to merge this and the next point:

> +
> ++++
> +*** Support Display Errors Indicator in margin
> +This allow indicate errors in both graphical and text display.
> +
>  +++
>  *** New user option 'flymake-show-diagnostics-at-end-of-line'.
>  When non-nil, Flymake shows summarized descriptions of diagnostics at
> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
> index db00cc59c0e..835f2057109 100644
> --- a/lisp/progmodes/flymake.el
> +++ b/lisp/progmodes/flymake.el
> @@ -180,6 +180,73 @@ See `flymake-error-bitmap' and `flymake-warning-bitmap'."
>  		 (const right-fringe)
>  		 (const :tag "No fringe indicators" nil)))
>  
> +(defcustom flymake-indicator-type 'fringes
> +  "Indicate which indicator type to use for display errors.
> +
> +The value can be nil (dont indicate errors but just highlight them),
> +fringes (use fringes) or margins (use margins)
> +
> +Difference between fringes and margin is that fringes support diplaying
> +bitmaps on graphical displays and margins display text in a blank area
> +of buffer that works in both graphical and text displays.
> +
> +See Info node `Fringes' and Info node `(elisp)Display Margins'."
> +  :version "30.1"
> +  :type '(choice (const :tag "Use Fringes" fringes)
> +		 (const :tag "Use Margins "margins)
> +		 (const :tag "No indicators" nil)))

There are tabs here.

> +
> +(defcustom flymake-error-margin-string '("‼" compilation-error)
> +  "String used in the margn for indicating errors.
> +The value may also be a list of two elements where the second
> +element specifies the face for the string.
> +See also `flymake-warning-margin-string'.
> +
> +The option `flymake-margin-indicator-position' controls how and where
> +this is used."
> +  :version "30.1"
> +  :type '(choice (string :tag "String")
> +                 (list :tag "String and face"
> +                       (string :tag "String")
> +                       (face :tag "Face"))))
> +
> +(defcustom flymake-warning-margin-string '("!" compilation-warning)
> +  "String used in the margin for indicating warnings.
> +The value may also be a list of two elements where the second
> +element specifies the face for the string.
> +See also `flymake-error-margin-string'.
> +
> +The option `flymake-margin-indicator-position' controls how and where
> +this is used."
> +  :version "30.1"
> +  :type '(choice (string :tag "String")
> +                 (list :tag "String and face"
> +                       (string :tag "String")
> +                       (face :tag "Face"))))
> +
> +(defcustom flymake-note-margin-string '("!" compilation-info)
> +  "String used in the margin for indicating info notes.
> +The value may also be a list of two elements where the second
> +element specifies the face for the string.
> +See also `flymake-error-margin-string'.
> +
> +The option `flymake-margin-indicator-position' controls how and where
> +this is used."
> +  :version "30.1"
> +  :type '(choice (string :tag "String")
> +                 (list :tag "String and face"
> +                       (string :tag "String")
> +                       (face :tag "Face"))))

Should or could these be merged into a single user option, that would
then have the form

  '((error "‼" compilation-error)
    (note "!" compilation-warning)
    (info "!" compilation-info))

> +
> +(defcustom flymake-margin-indicator-position 'left-margin
> +  "The position to put Flymake margin indicator.
> +The value can be nil (do not use indicators), `left-margin' or `right-margin'.
> +See `flymake-error-margin-string' and `flymake-warning-margin-string'."
> +  :version "30.1"
> +  :type '(choice (const left-margin)
> +		 (const right-margin)
> +		 (const :tag "No margin indicators" nil)))
> +
>  (make-obsolete-variable 'flymake-start-syntax-check-on-newline
>  		        "can check on newline in post-self-insert-hook"
>                          "27.1")
> @@ -630,6 +697,7 @@ Node `(Flymake)Flymake error types'"
>  
>  (put 'flymake-error 'face 'flymake-error)
>  (put 'flymake-error 'flymake-bitmap 'flymake-error-bitmap)
> +(put 'flymake-error 'flymake-margin-string 'flymake-error-margin-string)
>  (put 'flymake-error 'severity (warning-numeric-level :error))
>  (put 'flymake-error 'mode-line-face 'flymake-error-echo)
>  (put 'flymake-error 'echo-face 'flymake-error-echo)
> @@ -638,6 +706,7 @@ Node `(Flymake)Flymake error types'"
>  
>  (put 'flymake-warning 'face 'flymake-warning)
>  (put 'flymake-warning 'flymake-bitmap 'flymake-warning-bitmap)
> +(put 'flymake-warning 'flymake-margin-string 'flymake-warning-margin-string)
>  (put 'flymake-warning 'severity (warning-numeric-level :warning))
>  (put 'flymake-warning 'mode-line-face 'flymake-warning-echo)
>  (put 'flymake-warning 'echo-face 'flymake-warning-echo)
> @@ -646,6 +715,7 @@ Node `(Flymake)Flymake error types'"
>  
>  (put 'flymake-note 'face 'flymake-note)
>  (put 'flymake-note 'flymake-bitmap 'flymake-note-bitmap)
> +(put 'flymake-note 'flymake-margin-string 'flymake-note-margin-string)
>  (put 'flymake-note 'severity (warning-numeric-level :debug))
>  (put 'flymake-note 'mode-line-face 'flymake-note-echo)
>  (put 'flymake-note 'echo-face 'flymake-note-echo)
> @@ -682,19 +752,34 @@ associated `flymake-category' return DEFAULT."
>    (flymake--lookup-type-property type 'severity
>                                   (warning-numeric-level :error)))
>  
> -(defun flymake--fringe-overlay-spec (bitmap &optional recursed)
> -  (if (and (symbolp bitmap)
> -           (boundp bitmap)
> -           (not recursed))
> -      (flymake--fringe-overlay-spec
> -       (symbol-value bitmap) t)
> -    (and flymake-fringe-indicator-position
> -         bitmap
> -         (propertize "!" 'display
> -                     (cons flymake-fringe-indicator-position
> -                           (if (listp bitmap)
> -                               bitmap
> -                             (list bitmap)))))))
> +(defun flymake--fringe-overlay-spec (indicator)
> +  "Return INDICATOR-TYPE as propertized string to use in error indicators"
> +  (let* ((value (if (symbolp indicator)
> +                    (symbol-value indicator)
> +                  indicator))
> +         (indicator-car (if (listp value)
> +                            (car value)
> +                          value))
> +         (indicator-cdr (if (listp value)
> +                            (cdr value))))
> +    (cond
> +     ((symbolp indicator-car)
> +      (propertize "!" 'display
> +                  (cons flymake-fringe-indicator-position
> +                        (if (listp value)
> +                            value
> +                          (list value)))))
> +     ((stringp indicator-car)
> +      (propertize "!"
> +                  'display
> +                  `((margin ,flymake-margin-indicator-position)
> +                    ,(propertize
> +                      indicator-car
> +                      'face
> +                      `(:inherit (,indicator-cdr
> +                                  default)
> +                                 )))))

You should fold these parentheses back onto the previous line.

> +     (t nil))))

This is not needed, as cond will default to nil if no case matched.

>  
>  (defun flymake--equal-diagnostic-p (a b)
>    "Tell if A and B are equivalent `flymake--diag' objects."
> @@ -843,7 +928,11 @@ Return nil or the overlay created."
>          (flymake--fringe-overlay-spec
>           (flymake--lookup-type-property
>            type
> -          'flymake-bitmap
> +          (cond ((eq flymake-indicator-type 'fringes)
> +                 'flymake-bitmap)
> +                ((eq flymake-indicator-type 'margins)
> +                 'flymake-margin-string)
> +                (t nil))
>            (alist-get 'bitmap (alist-get type ; backward compat
>                                          flymake-diagnostic-types-alist)))))
>        ;; (default-maybe 'after-string

-- 
	Philip Kaludercic on peregrine