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