unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Flymake Support Indicator Errors in Margin
@ 2024-03-11 23:18 Elijah G
  2024-03-12  9:24 ` Philip Kaludercic
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-11 23:18 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 351 bytes --]

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.

[-- Attachment #1.2: Type: text/html, Size: 526 bytes --]

[-- Attachment #2: 0001-Flymake-Support-Indicator-Errors-in-Margin.patch --]
[-- Type: application/octet-stream, Size: 7976 bytes --]

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),
+fringes (use fringes) or margins (use margins).
+
++++
+*** 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)))
+
+(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"))))
+
+(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)
+                                 )))))
+     (t nil))))
 
 (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
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-11 23:18 [PATCH] Flymake Support Indicator Errors in Margin Elijah G
@ 2024-03-12  9:24 ` Philip Kaludercic
  2024-03-12 17:22   ` Elijah G
  0 siblings, 1 reply; 31+ messages in thread
From: Philip Kaludercic @ 2024-03-12  9:24 UTC (permalink / raw)
  To: Elijah G; +Cc: emacs-devel

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



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-12  9:24 ` Philip Kaludercic
@ 2024-03-12 17:22   ` Elijah G
  2024-03-13 12:18     ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-12 17:22 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 10344 bytes --]

 Thank you, I've fixed my patch.
Also I merged every flymake *-bitmap variable
into a single user option.

Also sorry for duplicating my reponses, I'm new in the mailing list.

On Tue, Mar 12, 2024 at 3:24 AM Philip Kaludercic <philipk@posteo.net>
wrote:

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

[-- Attachment #1.2: Type: text/html, Size: 13491 bytes --]

[-- Attachment #2: 0001-Flymake-Support-Indicator-Errors-in-Margin.patch --]
[-- Type: application/octet-stream, Size: 9482 bytes --]

From 4604566317ec4251231eeb06b331aada86e78ee7 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

---
 lisp/progmodes/flymake.el | 144 ++++++++++++++++++++++++--------------
 1 file changed, 90 insertions(+), 54 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index db00cc59c0e..a812a2edc7b 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -128,46 +128,21 @@
   :link '(custom-manual "(flymake) Top")
   :group 'tools)
 
-(defcustom flymake-error-bitmap '(flymake-double-exclamation-mark
-                                  compilation-error)
-  "Bitmap (a symbol) used in the fringe for indicating errors.
-The value may also be a list of two elements where the second
-element specifies the face for the bitmap.  For possible bitmap
-symbols, see `fringe-bitmaps'.  See also `flymake-warning-bitmap'.
+(defcustom flymake-fringe-indicators-bitmap
+  '((error flymake-double-exclamation-mark compilation-error)
+    (warning exclamation-mark compilation-warning)
+    (note exclamation-mark compilation-info))
+  "Bitmap (a symbol) used for fringes indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, its bitmap to use and its face,
+or a list of 2 elements specifying only the error type and its bitmap.
 
 The option `flymake-fringe-indicator-position' controls how and where
 this is used."
-  :version "24.3"
-  :type '(choice (symbol :tag "Bitmap")
-                 (list :tag "Bitmap and face"
-                       (symbol :tag "Bitmap")
-                       (face :tag "Face"))))
-
-(defcustom flymake-warning-bitmap '(exclamation-mark compilation-warning)
-  "Bitmap (a symbol) used in the fringe for indicating warnings.
-The value may also be a list of two elements where the second
-element specifies the face for the bitmap.  For possible bitmap
-symbols, see `fringe-bitmaps'.  See also `flymake-error-bitmap'.
-
-The option `flymake-fringe-indicator-position' controls how and where
-this is used."
-  :version "24.3"
-  :type '(choice (symbol :tag "Bitmap")
-                 (list :tag "Bitmap and face"
-                       (symbol :tag "Bitmap")
-                       (face :tag "Face"))))
-
-(defcustom flymake-note-bitmap '(exclamation-mark compilation-info)
-  "Bitmap (a symbol) used in the fringe for indicating info notes.
-The value may also be a list of two elements where the second
-element specifies the face for the bitmap.  For possible bitmap
-symbols, see `fringe-bitmaps'.  See also `flymake-error-bitmap'.
-
-The option `flymake-fringe-indicator-position' controls how and where
-this is used."
-  :version "26.1"
-  :type '(choice (symbol :tag "Bitmap")
-                 (list :tag "Bitmap and face"
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
                        (symbol :tag "Bitmap")
                        (face :tag "Face"))))
 
@@ -180,6 +155,48 @@ 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)))
+
+(defcustom flymake-margin-indicators-string '((error "‼" compilation-error)
+                                              (warning "!" compilation-warning)
+                                              (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, its string to use and its face,
+or a list of 2 elements specifying only the error type and its string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(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")
@@ -629,7 +646,8 @@ Node `(Flymake)Flymake error types'"
  "27.1")
 
 (put 'flymake-error 'face 'flymake-error)
-(put 'flymake-error 'flymake-bitmap 'flymake-error-bitmap)
+(put 'flymake-error 'flymake-bitmap (alist-get 'error flymake-fringe-indicators-bitmap))
+(put 'flymake-error 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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)
@@ -637,7 +655,8 @@ Node `(Flymake)Flymake error types'"
 (put 'flymake-error 'flymake-type-name "error")
 
 (put 'flymake-warning 'face 'flymake-warning)
-(put 'flymake-warning 'flymake-bitmap 'flymake-warning-bitmap)
+(put 'flymake-warning 'flymake-bitmap (alist-get 'warning flymake-fringe-indicators-bitmap))
+(put 'flymake-warning 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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)
@@ -645,7 +664,8 @@ Node `(Flymake)Flymake error types'"
 (put 'flymake-warning 'flymake-type-name "warning")
 
 (put 'flymake-note 'face 'flymake-note)
-(put 'flymake-note 'flymake-bitmap 'flymake-note-bitmap)
+(put 'flymake-note 'flymake-bitmap (alist-get 'note flymake-fringe-indicators-bitmap))
+(put 'flymake-note 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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 +702,32 @@ 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 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)))))))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -843,7 +876,10 @@ 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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-12 17:22   ` Elijah G
@ 2024-03-13 12:18     ` Eli Zaretskii
  2024-03-14  1:50       ` Elijah G
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2024-03-13 12:18 UTC (permalink / raw)
  To: Elijah G; +Cc: philipk, emacs-devel

> From: Elijah G <eg642616@gmail.com>
> Date: Tue, 12 Mar 2024 11:22:04 -0600
> Cc: emacs-devel@gnu.org
> 
> Thank you, I've fixed my patch.

Thanks, I have a few more comments below.

> Also I merged every flymake *-bitmap variable 
> into a single user option. 

I think this is a mistake: it makes this change backward-incompatible,
in the sense that users who had customizations of the options you are
removing will now have to rework their customizations.  We try to
avoid backward-incompatible changes like this as much as possible.  It
doesn't sound to me like this part of the patch is strictly needed, is
it?

> +  :type '(choice (const :tag "Use Fringes" fringes)
> +                 (const :tag "Use Margins "margins)
                                            ^^
A typo there.

> +(defcustom flymake-margin-indicators-string '((error "‼" compilation-error)

This is a non-ASCII character, so we should either use its ASCII
equivalent "!!" or test the display for being able to show it on the
screen (since it is likely the margin indicator will be used on TTY
frames).

Please also accompany your patch with the ChangeLog-style commit log
message (see the file CONTRIBUTE for the details).



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-13 12:18     ` Eli Zaretskii
@ 2024-03-14  1:50       ` Elijah G
  2024-03-14 11:05         ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-14  1:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: philipk, emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1753 bytes --]

On Wed, Mar 13, 2024 at 6:18 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Elijah G <eg642616@gmail.com>
> > Date: Tue, 12 Mar 2024 11:22:04 -0600
> > Cc: emacs-devel@gnu.org
> >
> > Thank you, I've fixed my patch.
>
> Thanks, I have a few more comments below.
>
> > Also I merged every flymake *-bitmap variable
> > into a single user option.
>
> I think this is a mistake: it makes this change backward-incompatible,
> in the sense that users who had customizations of the options you are
> removing will now have to rework their customizations.  We try to
> avoid backward-incompatible changes like this as much as possible.  It
> doesn't sound to me like this part of the patch is strictly needed, is
> it?
>

You are right, i was planning something like marking those variables as
deprecated for 30,
but since it makes backward incompatible think it can be too early to do it
in this patch,
However I find merging them easier to customize instead of defining them
separately.

If these variables are necessary, I've now reverted the changes in the
patch.

> +  :type '(choice (const :tag "Use Fringes" fringes)
> > +                 (const :tag "Use Margins "margins)
>                                             ^^
> A typo there.
>

Thanks, I've fixed it.

> +(defcustom flymake-margin-indicators-string '((error "‼"
> compilation-error)
>
> This is a non-ASCII character, so we should either use its ASCII
> equivalent "!!" or test the display for being able to show it on the
> screen (since it is likely the margin indicator will be used on TTY
> frames).
>
> Please also accompany your patch with the ChangeLog-style commit log
> message (see the file CONTRIBUTE for the details).
>

[-- Attachment #1.2: Type: text/html, Size: 2720 bytes --]

[-- Attachment #2: 0001-Flymake-Support-Indicator-Errors-in-Margin.patch --]
[-- Type: application/octet-stream, Size: 6809 bytes --]

From f02877c60337fd873fb04af2563e063d05762d48 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

Add support for displaying flymake errors in margin which
allows displaying in both graphical and text displays

* lisp/progmodes/flymake.el (flymake-margin-indicators-string,
flymake-indicator-type): New.
(flymake--fringe-overlay-spec, flymake--equal-diagnostic-p): Rework.
(flymake-error, flymake-warning, flymake-note): Use new margin value.
---
 lisp/progmodes/flymake.el | 93 +++++++++++++++++++++++++++++++++------
 1 file changed, 79 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index db00cc59c0e..da7e0079fc6 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -180,6 +180,52 @@ 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)))
+
+(defcustom flymake-margin-indicators-string
+  `((error ,(if (char-displayable-p 8252)
+               "‼"
+             "!!")
+     compilation-error)
+    (warning "!" compilation-warning)
+    (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, its string to use and its face,
+or a list of 2 elements specifying only the error type and its string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(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 +676,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 (alist-get 'error flymake-margin-indicators-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 +685,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 (alist-get 'warning flymake-margin-indicators-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 +694,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 (alist-get 'note flymake-margin-indicators-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 +731,32 @@ 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 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)))))))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -843,7 +905,10 @@ 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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-14  1:50       ` Elijah G
@ 2024-03-14 11:05         ` Eli Zaretskii
  2024-03-14 11:28           ` João Távora
  2024-03-14 15:35           ` Elijah G
  0 siblings, 2 replies; 31+ messages in thread
From: Eli Zaretskii @ 2024-03-14 11:05 UTC (permalink / raw)
  To: Elijah G, João Távora; +Cc: philipk, emacs-devel

> From: Elijah G <eg642616@gmail.com>
> Date: Wed, 13 Mar 2024 19:50:59 -0600
> Cc: philipk@posteo.net, emacs-devel@gnu.org
> 
>  I think this is a mistake: it makes this change backward-incompatible,
>  in the sense that users who had customizations of the options you are
>  removing will now have to rework their customizations.  We try to
>  avoid backward-incompatible changes like this as much as possible.  It
>  doesn't sound to me like this part of the patch is strictly needed, is
>  it?
> 
>  
> You are right, i was planning something like marking those variables as deprecated for 30,
> but since it makes backward incompatible think it can be too early to do it in this patch,
> However I find merging them easier to customize instead of defining them separately.
> 
> If these variables are necessary, I've now reverted the changes in the patch.

Thanks.

> +(defcustom flymake-margin-indicators-string
> +  `((error ,(if (char-displayable-p 8252)
> +               "‼"
> +             "!!")
> +     compilation-error)

When do you expect this value to be evaluated?

In general, I wonder whether it would be simpler and wiser to use just
"!!", and leave it to users to customize to "‼" if their displays
support that.  (We can mention the possibility in the doc string.)

João, any comments?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-14 11:05         ` Eli Zaretskii
@ 2024-03-14 11:28           ` João Távora
  2024-03-14 15:35           ` Elijah G
  1 sibling, 0 replies; 31+ messages in thread
From: João Távora @ 2024-03-14 11:28 UTC (permalink / raw)
  To: Eli Zaretskii, Spencer Baugh; +Cc: Elijah G, philipk, emacs-devel

On Thu, Mar 14, 2024 at 11:05 AM Eli Zaretskii <eliz@gnu.org> wrote:

> João, any comments?

I'm CCing Spencer Baugh, who has volunteered to become Flymake
maintainer and who I think will do a fine job.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-14 11:05         ` Eli Zaretskii
  2024-03-14 11:28           ` João Távora
@ 2024-03-14 15:35           ` Elijah G
  2024-03-16 11:10             ` Eli Zaretskii
  1 sibling, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-14 15:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: João Távora, philipk, emacs-devel

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

On Thu, Mar 14, 2024 at 5:05 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > +(defcustom flymake-margin-indicators-string
> > +  `((error ,(if (char-displayable-p 8252)
> > +               "‼"
> > +             "!!")
> > +     compilation-error)
>
> When do you expect this value to be evaluated?
>
> In general, I wonder whether it would be simpler and wiser to use just
> "!!", and leave it to users to customize to "‼" if their displays
> support that.  (We can mention the possibility in the doc string.)

I implemented it in a similar way to how display-fill-column-indicator
does with display-fill-column-indicator-character, this since I expect
users to only set left-margin-width to 1, because if there is several
errors on the same line, the indicators will be displayed next to each
other and it could be confusing.

However I agree that it is better to change it to just "!!",
I fixed the patch and added a doc string for the possibility of including
non-ASCII characters.

[-- Attachment #2: 0001-Flymake-Support-Indicator-Errors-in-Margin.patch --]
[-- Type: application/octet-stream, Size: 6810 bytes --]

From f02877c60337fd873fb04af2563e063d05762d48 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

Add support for displaying flymake errors in margin which
allows displaying in both graphical and text displays.

* lisp/progmodes/flymake.el (flymake-margin-indicators-string,
flymake-indicator-type): New.
(flymake--fringe-overlay-spec, flymake--equal-diagnostic-p): Rework.
(flymake-error, flymake-warning, flymake-note): Use new margin value.
---
 lisp/progmodes/flymake.el | 93 +++++++++++++++++++++++++++++++++------
 1 file changed, 79 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index db00cc59c0e..da7e0079fc6 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -180,6 +180,51 @@ 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)))
+
+(defcustom flymake-margin-indicators-string
+  '((error "!!" compilation-error)
+    (warning "!" compilation-warning)
+    (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, its string to use and its face,
+or a list of 2 elements specifying only the error type and its string.
+
+The indicators can be both ASCII and non-ASCII (if supported) string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(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 +676,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 (alist-get 'error flymake-margin-indicators-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 +685,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 (alist-get 'warning flymake-margin-indicators-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 +694,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 (alist-get 'note flymake-margin-indicators-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 +731,32 @@ 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 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)))))))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -843,7 +905,10 @@ 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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-14 15:35           ` Elijah G
@ 2024-03-16 11:10             ` Eli Zaretskii
  2024-03-17 16:44               ` bird
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2024-03-16 11:10 UTC (permalink / raw)
  To: Elijah G, Spencer Baugh; +Cc: joaotavora, philipk, emacs-devel

> From: Elijah G <eg642616@gmail.com>
> Date: Thu, 14 Mar 2024 09:35:33 -0600
> Cc: João Távora <joaotavora@gmail.com>, 
> 	philipk@posteo.net, emacs-devel@gnu.org
> 
> On Thu, Mar 14, 2024 at 5:05 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > > +(defcustom flymake-margin-indicators-string
> > > +  `((error ,(if (char-displayable-p 8252)
> > > +               "‼"
> > > +             "!!")
> > > +     compilation-error)
> >
> > When do you expect this value to be evaluated?
> >
> > In general, I wonder whether it would be simpler and wiser to use just
> > "!!", and leave it to users to customize to "‼" if their displays
> > support that.  (We can mention the possibility in the doc string.)
> 
> I implemented it in a similar way to how display-fill-column-indicator
> does with display-fill-column-indicator-character, this since I expect
> users to only set left-margin-width to 1, because if there is several
> errors on the same line, the indicators will be displayed next to each
> other and it could be confusing.
> 
> However I agree that it is better to change it to just "!!",
> I fixed the patch and added a doc string for the possibility of including
> non-ASCII characters.

Spencer, any comments?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-16 11:10             ` Eli Zaretskii
@ 2024-03-17 16:44               ` bird
  2024-03-17 17:01                 ` Eli Zaretskii
                                   ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: bird @ 2024-03-17 16:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Elijah G, Spencer Baugh, joaotavora, philipk, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Elijah G <eg642616@gmail.com>
>> Date: Thu, 14 Mar 2024 09:35:33 -0600
>> Cc: João Távora <joaotavora@gmail.com>, 
>> 	philipk@posteo.net, emacs-devel@gnu.org
>> 
>> On Thu, Mar 14, 2024 at 5:05 AM Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> > > +(defcustom flymake-margin-indicators-string
>> > > +  `((error ,(if (char-displayable-p 8252)
>> > > +               "‼"
>> > > +             "!!")
>> > > +     compilation-error)
>> >
>> > When do you expect this value to be evaluated?
>> >
>> > In general, I wonder whether it would be simpler and wiser to use just
>> > "!!", and leave it to users to customize to "‼" if their displays
>> > support that.  (We can mention the possibility in the doc string.)
>> 
>> I implemented it in a similar way to how display-fill-column-indicator
>> does with display-fill-column-indicator-character, this since I expect
>> users to only set left-margin-width to 1, because if there is several
>> errors on the same line, the indicators will be displayed next to each
>> other and it could be confusing.
>> 
>> However I agree that it is better to change it to just "!!",
>> I fixed the patch and added a doc string for the possibility of including
>> non-ASCII characters.
>
> Spencer, any comments?

A few thoughts:

- I looked through packages in core which use fringes.  The only
  function which seems to also support margins is
  gdb-put-breakpoint-icon, which automatically uses margins instead of
  fringes to display a breakpoint icon if (display-images-p) is nil.

- The current version of Elijah's patch requires the user to explicitly
  configure left-margin-width (or right-margin-width) to non-zero if
  they want the indicators to appear; the margins are not automatically
  grown, unlike in gdb-put-breakpoint-icon.

- I'm not sure whether or why this is even desirable in a TTY.

  The indicators duplicate information that's already communicated by
  the face, so they're a minor benefit.  In a graphical frame, the
  indicators are basically "free" in terms of visual space, since they
  display in the fringe which already exists, so it's worth having them.

  But in a TTY, with the current patch, two columns of text in every
  flymake buffer will be devoted to displaying these indicators, whether
  they're currently needed or not.  That seems like too high a cost,
  unless I'm missing something.

- If it *is* desirable on a TTY, then the need to explicitly configure
  it is unfortunate.

- One alternative design could display the indicators in the first few
  columns of text, if that would otherwise be whitespace, to avoid
  wasting space for the margin.  I think I've seen other packages which
  display stuff in the whitespace in the first column.

- If it's just about making diagnostics more visible, there are various
  things we could do, like displaying an indicator inline with the text
  which has the diagnostic, or highlighting the entire line containing
  the diagnostic.

Elijah, could you say more about your setup?

- You mentioned you already have a left margin configured; what is
  displayed in that margin? Is it anything other than flymake/flycheck
  indicators?

- Do you have the margin automatically disappear if you aren't using
  flycheck in a buffer/if there are no flycheck errors in a buffer?

- Is there some reason that seeing the indicators is especially
  important for you?  Does it communicate information not already
  carried by the face?

- If you just want the diagnostics to be more visible, would customizing
  the face on the diagnostics help?  Or perhaps customizing
  flymake-show-diagnostics-at-end-of-line?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 16:44               ` bird
@ 2024-03-17 17:01                 ` Eli Zaretskii
  2024-03-17 17:34                 ` Elijah G
  2024-03-17 17:49                 ` Elijah G
  2 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2024-03-17 17:01 UTC (permalink / raw)
  To: bird; +Cc: eg642616, sbaugh, joaotavora, philipk, emacs-devel

> From: bird <sbaugh@catern.com>
> Date: Sun, 17 Mar 2024 16:44:15 +0000 (UTC)
> Cc: Elijah G <eg642616@gmail.com>, Spencer Baugh <sbaugh@janestreet.com>,
> 	joaotavora@gmail.com, philipk@posteo.net, emacs-devel@gnu.org
> 
> - I'm not sure whether or why this is even desirable in a TTY.
> 
>   The indicators duplicate information that's already communicated by
>   the face, so they're a minor benefit.  In a graphical frame, the
>   indicators are basically "free" in terms of visual space, since they
>   display in the fringe which already exists, so it's worth having them.
> 
>   But in a TTY, with the current patch, two columns of text in every
>   flymake buffer will be devoted to displaying these indicators, whether
>   they're currently needed or not.  That seems like too high a cost,
>   unless I'm missing something.

As long as this is an opt-in feature, I don't see this as a problem.

> - If it *is* desirable on a TTY, then the need to explicitly configure
>   it is unfortunate.

I tend to agree.  And not only on TTY frames: GUI frames can disable
fringes as well.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 16:44               ` bird
  2024-03-17 17:01                 ` Eli Zaretskii
@ 2024-03-17 17:34                 ` Elijah G
  2024-03-17 18:43                   ` bird
  2024-03-19  7:03                   ` Augusto Stoffel
  2024-03-17 17:49                 ` Elijah G
  2 siblings, 2 replies; 31+ messages in thread
From: Elijah G @ 2024-03-17 17:34 UTC (permalink / raw)
  To: bird; +Cc: Eli Zaretskii, Spencer Baugh, joaotavora, philipk, emacs-devel

On Sun, Mar 17, 2024 at 10:44 AM bird <sbaugh@catern.com> wrote:
> Elijah, could you say more about your setup?
>
> - You mentioned you already have a left margin configured; what is
>   displayed in that margin? Is it anything other than flymake/flycheck
>   indicators?

Yes, it is dape-mode, in terminal dape is automatically configured for
display breakpoints in left-margin, however the error indicators can appear
next to breakpoint or even hide it if left-margin-width is less than 1.
This can also apply to other packages that can use margins like
git-gutter, et cetera.

> - Do you have the margin automatically disappear if you aren't using
>   flycheck in a buffer/if there are no flycheck errors in a buffer?

Yes, I do this with setq-local.

> - Is there some reason that seeing the indicators is especially
>   important for you?  Does it communicate information not already
>   carried by the face?

It is due that the fringes are too small in HIDPI screens, i know
I can change it to a bigger fringe bitmap and increase fringe width size,
however this is a problem when increasing and decreasing text size.

Also using margins allows a wide variety of characters to be used, especially
non-ASCII.

> - If you just want the diagnostics to be more visible, would customizing
>   the face on the diagnostics help?  Or perhaps customizing
>   flymake-show-diagnostics-at-end-of-line?

When flymake-show-diagnostics-at-end-of-line is enabled, it makes
harder (for me) see where i am, especially when there are multiple errors in the
same or in multiple lines, it can be annoying, however using margins doesn't
cut text and it is easier to see where errors are.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 16:44               ` bird
  2024-03-17 17:01                 ` Eli Zaretskii
  2024-03-17 17:34                 ` Elijah G
@ 2024-03-17 17:49                 ` Elijah G
  2024-03-19  7:04                   ` Augusto Stoffel
  2 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-17 17:49 UTC (permalink / raw)
  To: bird; +Cc: Eli Zaretskii, Spencer Baugh, joaotavora, philipk, emacs-devel

On Sun, Mar 17, 2024 at 10:44 AM bird <sbaugh@catern.com> wrote:
> - The current version of Elijah's patch requires the user to explicitly
>   configure left-margin-width (or right-margin-width) to non-zero if
>   they want the indicators to appear; the margins are not automatically
>   grown, unlike in gdb-put-breakpoint-icon.

If you all like, I can update the patch to implement something like that.

> - If it's just about making diagnostics more visible, there are various
>   things we could do, like displaying an indicator inline with the text
>   which has the diagnostic, or highlighting the entire line containing
>   the diagnostic.

I was thinking of implementing things like that in separated patches.

Also as Eli wrote, I only expect to make this as an optional feature.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 17:34                 ` Elijah G
@ 2024-03-17 18:43                   ` bird
  2024-03-17 19:21                     ` Elijah G
  2024-03-19  7:03                   ` Augusto Stoffel
  1 sibling, 1 reply; 31+ messages in thread
From: bird @ 2024-03-17 18:43 UTC (permalink / raw)
  To: Elijah G; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

Elijah G <eg642616@gmail.com> writes:
> On Sun, Mar 17, 2024 at 10:44 AM bird <sbaugh@catern.com> wrote:
>> Elijah, could you say more about your setup?
>>
>> - You mentioned you already have a left margin configured; what is
>>   displayed in that margin? Is it anything other than flymake/flycheck
>>   indicators?
>
> Yes, it is dape-mode, in terminal dape is automatically configured for
> display breakpoints in left-margin, however the error indicators can appear
> next to breakpoint or even hide it if left-margin-width is less than 1.
> This can also apply to other packages that can use margins like
> git-gutter, et cetera.

I see.  Wouldn't the margin-based indicator conflict with these
packages, then?

>> - Is there some reason that seeing the indicators is especially
>>   important for you?  Does it communicate information not already
>>   carried by the face?
>
> It is due that the fringes are too small in HIDPI screens, i know
> I can change it to a bigger fringe bitmap and increase fringe width size,
> however this is a problem when increasing and decreasing text size.

Oh, so the motivation for this patch is actually for hidpi support in
graphical frames, not support in terminal frames?

I see that there are a number of bugs about fringes not being scaled for
hidpi screens on X... that's unfortunate.  Especially relevant is
bug#37932.

Po Lu, any comments about supporting hidpi fringes on X?  Is that
fundamentally hard, or is it just a matter of writing some code which
does scaling?

Even a basic, partial implementation of hidpi fringes would be very
good, I think.  Without that, perhaps flymake should use margins instead
of fringes even on graphical frames.  hidpi screens are increasingly
common, after all.




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 18:43                   ` bird
@ 2024-03-17 19:21                     ` Elijah G
  2024-03-25  1:46                       ` Elijah G
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-17 19:21 UTC (permalink / raw)
  To: bird; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

On Sun, Mar 17, 2024 at 12:43 PM bird <sbaugh@catern.com> wrote:
>
> Elijah G <eg642616@gmail.com> writes:
> > On Sun, Mar 17, 2024 at 10:44 AM bird <sbaugh@catern.com> wrote:
> >> Elijah, could you say more about your setup?
> >>
> >> - You mentioned you already have a left margin configured; what is
> >>   displayed in that margin? Is it anything other than flymake/flycheck
> >>   indicators?
> >
> > Yes, it is dape-mode, in terminal dape is automatically configured for
> > display breakpoints in left-margin, however the error indicators can appear
> > next to breakpoint or even hide it if left-margin-width is less than 1.
> > This can also apply to other packages that can use margins like
> > git-gutter, et cetera.
>
> I see.  Wouldn't the margin-based indicator conflict with these
> packages, then?

I don't think so, users can increase margin width and see others
indicators in the margin, also it's the same problem with fringes
because any package can
overwrite what is displayed there, however in margin the user has the
ability to see
the rest.

> >> - Is there some reason that seeing the indicators is especially
> >>   important for you?  Does it communicate information not already
> >>   carried by the face?
> >
> > It is due that the fringes are too small in HIDPI screens, i know
> > I can change it to a bigger fringe bitmap and increase fringe width size,
> > however this is a problem when increasing and decreasing text size.
>
> Oh, so the motivation for this patch is actually for hidpi support in
> graphical frames, not support in terminal frames?

Both, supporting graphical and terminal frames.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 17:34                 ` Elijah G
  2024-03-17 18:43                   ` bird
@ 2024-03-19  7:03                   ` Augusto Stoffel
  1 sibling, 0 replies; 31+ messages in thread
From: Augusto Stoffel @ 2024-03-19  7:03 UTC (permalink / raw)
  To: Elijah G
  Cc: bird, Eli Zaretskii, Spencer Baugh, joaotavora, philipk,
	emacs-devel

On Sun, 17 Mar 2024 at 11:34, Elijah G wrote:

> When flymake-show-diagnostics-at-end-of-line is enabled, it makes
> harder (for me) see where i am, especially when there are multiple errors in the
> same or in multiple lines, it can be annoying, however using margins doesn't
> cut text and it is easier to see where errors are.

I guess there should be an option to truncate the EOL messages (same
effect as truncating long lines, but without affecting regular buffer
text).



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 17:49                 ` Elijah G
@ 2024-03-19  7:04                   ` Augusto Stoffel
  0 siblings, 0 replies; 31+ messages in thread
From: Augusto Stoffel @ 2024-03-19  7:04 UTC (permalink / raw)
  To: Elijah G
  Cc: bird, Eli Zaretskii, Spencer Baugh, joaotavora, philipk,
	emacs-devel

On Sun, 17 Mar 2024 at 11:49, Elijah G wrote:

>> - If it's just about making diagnostics more visible, there are various
>>   things we could do, like displaying an indicator inline with the text
>>   which has the diagnostic, or highlighting the entire line containing
>>   the diagnostic.
>
> I was thinking of implementing things like that in separated patches.

I haven't looked at the patch or closely followed this thread, but
something I would appreciate are more options to make the diagnostics
_less_ visible.  Maybe options to disable the underlining or show the
underline or EOL message only on the current line...



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-17 19:21                     ` Elijah G
@ 2024-03-25  1:46                       ` Elijah G
  2024-03-27  0:13                         ` sbaugh
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-25  1:46 UTC (permalink / raw)
  To: bird; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

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

I don't know if there's still interest in implementing this.
If so, I've fixed a little error in the patch.

[-- Attachment #2: 0001-Flymake-Support-Indicator-Errors-in-Margin.patch --]
[-- Type: application/x-patch, Size: 6760 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-25  1:46                       ` Elijah G
@ 2024-03-27  0:13                         ` sbaugh
  2024-03-27  0:36                           ` Elijah G
  2024-03-27 21:29                           ` Elijah G
  0 siblings, 2 replies; 31+ messages in thread
From: sbaugh @ 2024-03-27  0:13 UTC (permalink / raw)
  To: Elijah G; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

Elijah G <eg642616@gmail.com> writes:
> I don't know if there's still interest in implementing this.
> If so, I've fixed a little error in the patch.

My primary concern with the current state of the patch is that support
for HiDPI frames and terminal frames has to be explicitly configured by
the user - and when that support is configured, then "normal" graphical
frames have degraded behavior.  It would be better for it to be
automatic somehow.

Secondarily, the margins should be automatically resized to the needed
width - it seems unreasonable to require the user to explicitly
configure them as well, even if terminal support must be explicitly
configured.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-27  0:13                         ` sbaugh
@ 2024-03-27  0:36                           ` Elijah G
  2024-03-27 21:29                           ` Elijah G
  1 sibling, 0 replies; 31+ messages in thread
From: Elijah G @ 2024-03-27  0:36 UTC (permalink / raw)
  To: sbaugh; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

On Tue, Mar 26, 2024 at 6:13 PM <sbaugh@catern.com> wrote:
>
> Elijah G <eg642616@gmail.com> writes:
> > I don't know if there's still interest in implementing this.
> > If so, I've fixed a little error in the patch.
>
> My primary concern with the current state of the patch is that support
> for HiDPI frames and terminal frames has to be explicitly configured by
> the user - and when that support is configured, then "normal" graphical
> frames have degraded behavior.  It would be better for it to be
> automatic somehow.

About HiDPI i don't know if emacs has an option that determines
if the screen is HiDPI, i may let the user choose about that.
But for graphical frames you are right, I will update the patch to
automatically choose.

> Secondarily, the margins should be automatically resized to the needed
> width - it seems unreasonable to require the user to explicitly
> configure them as well, even if terminal support must be explicitly
> configured.

I think I may add another user option to let users choose if they want to
auto resize margins because it can override users setup.
I'll work on this too.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-27  0:13                         ` sbaugh
  2024-03-27  0:36                           ` Elijah G
@ 2024-03-27 21:29                           ` Elijah G
  2024-03-28  6:01                             ` Eli Zaretskii
                                               ` (2 more replies)
  1 sibling, 3 replies; 31+ messages in thread
From: Elijah G @ 2024-03-27 21:29 UTC (permalink / raw)
  To: sbaugh; +Cc: Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

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

I've now updated the patch to allow auto resize margins and using margins
when it's in terminal frames.

Sadly I couldn't find anything for detecting HDiPI frames, so the user
must have to configure it.

[-- Attachment #2: 0001-Flymake-support-for-indicating-errors-in-margin.patch --]
[-- Type: application/octet-stream, Size: 9680 bytes --]

From b8f4c14b868f94fcf4e5a4ea3a86900919b0b244 Mon Sep 17 00:00:00 2001
From: "Elias G. B. Perez" <eg642616@gmail.com>
Date: Wed, 27 Mar 2024 14:27:19 -0600
Subject: [PATCH] Flymake support for indicating errors in margin

Add optional support for display flymake error in margin that
allow displaying error indicators in terminal frames.
* lisp/progmodes/flymake.el (flymake-indicator-type):
(flymake-margin-indicators-string, flymake-autoresize-margins)
(flymake-margin-indicator-position): New user options.
(flymake-original-margin-width): Add buffer-local variable for
store original buffer margin width.
(flymake-error, flymake-warning, flymake-note): Use new margin
value.
(flymake--fringe-overlay-spec): Change function name.
(flymake--indicator-overlay-spec): New function.
(flymake--resize-margins): New function for resize margins in
current buffer.
(flymake--highlight-line, flymake-mode): Rework.
---
 lisp/progmodes/flymake.el | 131 +++++++++++++++++++++++++++++++++-----
 1 file changed, 116 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index db00cc59c0e..41750d56070 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -180,6 +180,58 @@ flymake-fringe-indicator-position
 		 (const right-fringe)
 		 (const :tag "No fringe indicators" nil)))
 
+(defcustom flymake-indicator-type (if (display-graphic-p)
+                                      'fringes
+                                    'margins)
+  "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)))
+
+(defcustom flymake-margin-indicators-string
+  '((error "!!" compilation-error)
+    (warning "!" compilation-warning)
+    (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, its string to use and its face,
+or a list of 2 elements specifying only the error type and its string.
+Indicators can be an ASCII or non-ASCII string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(defcustom flymake-autoresize-margins t
+  "If non-nil automatically resize margin-width.
+Only if `flymake-indicator-type' is set to margins."
+  :version "30.1"
+  :type 'boolean)
+
+(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-margin-indicators-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")
@@ -258,6 +310,9 @@ flymake-timer
 (defvar-local flymake-check-start-time nil
   "Time at which syntax check was started.")
 
+(defvar-local flymake-original-margin-width nil
+  "Store original margin width")
+
 (defun flymake--log-1 (level sublog msg &rest args)
   "Do actual work for `flymake-log'."
   (let (;; never popup the log buffer
@@ -630,6 +685,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-error 'face 'flymake-error)
 (put 'flymake-error 'flymake-bitmap 'flymake-error-bitmap)
+(put 'flymake-error 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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 +694,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-warning 'face 'flymake-warning)
 (put 'flymake-warning 'flymake-bitmap 'flymake-warning-bitmap)
+(put 'flymake-warning 'flymake-margin-string (alist-get 'warning flymake-margin-indicators-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 +703,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-note 'face 'flymake-note)
 (put 'flymake-note 'flymake-bitmap 'flymake-note-bitmap)
+(put 'flymake-note 'flymake-margin-string (alist-get 'note flymake-margin-indicators-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 +740,53 @@ flymake--severity
   (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--indicator-overlay-spec (indicator)
+  "Return INDICATOR 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)))))))))
+
+(defun flymake--resize-margins (&optional og-width)
+  "Resize current window margins according `flymake-margin-indicator-position'.
+Return to original margin width if OG-WIDTH is non-nil."
+  (when (and (eq flymake-indicator-type 'margins)
+             flymake-autoresize-margins)
+    (cond
+     ((and og-width flymake-original-margin-width)
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local left-margin-width flymake-original-margin-width)
+        (setq-local right-margin-width flymake-original-margin-width)))
+     (t
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local flymake-original-margin-width left-margin-width
+                      left-margin-width 2)
+        (setq-local flymake-original-margin-width right-margin-width
+                    right-margin-width 2))))
+    ;; Apply margin to all windows avalaibles
+    (mapc (lambda (x)
+            (set-window-buffer x (window-buffer x)))
+          (get-buffer-window-list nil nil 'visible))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -840,10 +932,13 @@ flymake--highlight-line
                                         type prop value)))))
       (default-maybe 'face 'flymake-error)
       (default-maybe 'before-string
-        (flymake--fringe-overlay-spec
+        (flymake--indicator-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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
@@ -1285,6 +1380,9 @@ flymake-mode
     (add-hook 'kill-buffer-hook 'flymake-kill-buffer-hook nil t)
     (add-hook 'eldoc-documentation-functions 'flymake-eldoc-function t t)
 
+    ;; AutoResize margins.
+    (flymake--resize-margins)
+
     ;; If Flymake happened to be already ON, we must cleanup
     ;; existing diagnostic overlays, lest we forget them by blindly
     ;; reinitializing `flymake--state' in the next line.
@@ -1333,6 +1431,9 @@ flymake-mode
     ;;+(remove-hook 'find-file-hook (function flymake-find-file-hook) t)
     (remove-hook 'eldoc-documentation-functions 'flymake-eldoc-function t)
 
+    ;; return margin to original size
+    (flymake--resize-margins t)
+
     (when flymake-timer
       (cancel-timer flymake-timer)
       (setq flymake-timer nil))
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-27 21:29                           ` Elijah G
@ 2024-03-28  6:01                             ` Eli Zaretskii
  2024-03-28 17:34                               ` Elijah G
  2024-03-28  7:30                             ` Juri Linkov
  2024-04-06 11:35                             ` Eli Zaretskii
  2 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2024-03-28  6:01 UTC (permalink / raw)
  To: Elijah G; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

> From: Elijah G <eg642616@gmail.com>
> Date: Wed, 27 Mar 2024 15:29:28 -0600
> Cc: Eli Zaretskii <eliz@gnu.org>, Spencer Baugh <sbaugh@janestreet.com>, philipk@posteo.net, 
> 	emacs-devel@gnu.org, Po Lu <luangruo@yahoo.com>
> 
> I've now updated the patch to allow auto resize margins and using margins
> when it's in terminal frames.

Thanks.  Will review soon, and I hope Spencer will as well.

> Sadly I couldn't find anything for detecting HDiPI frames, so the user
> must have to configure it.

Can you explain the problem, and point me to the code which has to
deal with it?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-27 21:29                           ` Elijah G
  2024-03-28  6:01                             ` Eli Zaretskii
@ 2024-03-28  7:30                             ` Juri Linkov
  2024-03-28 17:44                               ` Elijah G
  2024-04-06 11:35                             ` Eli Zaretskii
  2 siblings, 1 reply; 31+ messages in thread
From: Juri Linkov @ 2024-03-28  7:30 UTC (permalink / raw)
  To: Elijah G
  Cc: sbaugh, Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

> Sadly I couldn't find anything for detecting HDiPI frames, so the user
> must have to configure it.

If detecting HDiPI frames is required to get a margin width,
there is an example for outline--margin-width in outline.el.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-28  6:01                             ` Eli Zaretskii
@ 2024-03-28 17:34                               ` Elijah G
  2024-04-06 11:36                                 ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-03-28 17:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

On Thu, Mar 28, 2024 at 12:01 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > Sadly I couldn't find anything for detecting HDiPI frames, so the user
> > must have to configure it.
>
> Can you explain the problem, and point me to the code which has to
> deal with it?

Sure, in this part of the code.

> +(defcustom flymake-indicator-type (if (display-graphic-p)
> +                                      'fringes
> +                                    'margins)
> +  "Indicate which indicator type to use for display errors.

I wanted to check if the frame is in HiDPI to
automatically set to margins because fringes are so small in those screens.

I don't know if emacs has an option to check if the frame is HiDPI,
but I think it's better as it is if emacs will support fringes in HiDPI
in any next release.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-28  7:30                             ` Juri Linkov
@ 2024-03-28 17:44                               ` Elijah G
  0 siblings, 0 replies; 31+ messages in thread
From: Elijah G @ 2024-03-28 17:44 UTC (permalink / raw)
  To: Juri Linkov
  Cc: sbaugh, Eli Zaretskii, Spencer Baugh, philipk, emacs-devel, Po Lu

On Thu, Mar 28, 2024 at 1:56 AM Juri Linkov <juri@linkov.net> wrote:
>
> > Sadly I couldn't find anything for detecting HDiPI frames, so the user
> > must have to configure it.
>
> If detecting HDiPI frames is required to get a margin width,
> there is an example for outline--margin-width in outline.el.

Thanks, but detecting HiDPI is not for that,
margin width plays well in those screens according to font size,
so HiDPI isn't necessary to get margin width.

But for some users that have HiDPI screens it will be a problem
due fringes indicator and its bitmaps don't scale.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-27 21:29                           ` Elijah G
  2024-03-28  6:01                             ` Eli Zaretskii
  2024-03-28  7:30                             ` Juri Linkov
@ 2024-04-06 11:35                             ` Eli Zaretskii
  2024-04-06 20:14                               ` Elijah G
  2 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2024-04-06 11:35 UTC (permalink / raw)
  To: Elijah G; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

> From: Elijah G <eg642616@gmail.com>
> Date: Wed, 27 Mar 2024 15:29:28 -0600
> Cc: Eli Zaretskii <eliz@gnu.org>, Spencer Baugh <sbaugh@janestreet.com>, philipk@posteo.net, 
> 	emacs-devel@gnu.org, Po Lu <luangruo@yahoo.com>
> 
> I've now updated the patch to allow auto resize margins and using margins
> when it's in terminal frames.

Thanks, I think this is almost ready to be installed; see a few minor
comments below.

> Sadly I couldn't find anything for detecting HDiPI frames, so the user
> must have to configure it.

I don't think Emacs should second-guess the user in this matter: let
the user decide whether the fringes are too narrow for these
indicators.

> +(defcustom flymake-indicator-type (if (display-graphic-p)
> +                                      'fringes
> +                                    'margins)
> +  "Indicate which indicator type to use for display errors.
> +
> +The value can be nil (dont indicate errors but just highlight them),
                         ^^^^
"don't"

> +(defcustom flymake-margin-indicators-string
> +  '((error "!!" compilation-error)
> +    (warning "!" compilation-warning)
> +    (note "!" compilation-info))
> +  "Strings used for margins indicators.
> +The value of each list may be a list of 3 elements where specifies the
> +error type, its string to use and its face,
               ^^^^^^^^^^
"the string"

> +or a list of 2 elements specifying only the error type and its string.
                                                          ^^^^^^^^^^^^^^
"and the corresponding string"

> +Indicators can be an ASCII or non-ASCII string.

This sentence is redundant, so I suggest to remove it.  The ASCII vs
non-ASCII issue is only relevant for the default value; users can do
whatever they want when they customize the option.

> +(defcustom flymake-autoresize-margins t
> +  "If non-nil automatically resize margin-width.
                ^
Comma is missing there.  Also, how about telling what triggers
the resizing?

> +Only if `flymake-indicator-type' is set to margins."
   ^^^^^^^
"Only relevant if..."

> +(defvar-local flymake-original-margin-width nil
> +  "Store original margin width")

Should this be an internal variable (named flymake--original-margin-width)?
Also, please consider telling in the doc string when is the width restored.

> +(defun flymake--resize-margins (&optional og-width)
> +  "Resize current window margins according `flymake-margin-indicator-position'.
                                    ^^^^^^^^^
"according to"

> +Return to original margin width if OG-WIDTH is non-nil."

Suggest to rename OG-WIDTH to ORIG-WIDTH.

Last, but not least: please mention this change (and the new user
options) in NEWS and in the Flymake manual.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-03-28 17:34                               ` Elijah G
@ 2024-04-06 11:36                                 ` Eli Zaretskii
  0 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2024-04-06 11:36 UTC (permalink / raw)
  To: Elijah G; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

> From: Elijah G <eg642616@gmail.com>
> Date: Thu, 28 Mar 2024 11:34:23 -0600
> Cc: sbaugh@catern.com, sbaugh@janestreet.com, philipk@posteo.net, 
> 	emacs-devel@gnu.org, luangruo@yahoo.com
> 
> On Thu, Mar 28, 2024 at 12:01 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > > Sadly I couldn't find anything for detecting HDiPI frames, so the user
> > > must have to configure it.
> >
> > Can you explain the problem, and point me to the code which has to
> > deal with it?
> 
> Sure, in this part of the code.
> 
> > +(defcustom flymake-indicator-type (if (display-graphic-p)
> > +                                      'fringes
> > +                                    'margins)
> > +  "Indicate which indicator type to use for display errors.
> 
> I wanted to check if the frame is in HiDPI to
> automatically set to margins because fringes are so small in those screens.

As I wrote up-thread, I don't think we should adapt to HiDPI in this
respect.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-04-06 11:35                             ` Eli Zaretskii
@ 2024-04-06 20:14                               ` Elijah G
  2024-04-07  5:47                                 ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-04-06 20:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

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

On Sat, Apr 6, 2024 at 5:35 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Elijah G <eg642616@gmail.com>
> > Date: Wed, 27 Mar 2024 15:29:28 -0600
> > Cc: Eli Zaretskii <eliz@gnu.org>, Spencer Baugh <sbaugh@janestreet.com>, philipk@posteo.net,
> >       emacs-devel@gnu.org, Po Lu <luangruo@yahoo.com>
> >
> > I've now updated the patch to allow auto resize margins and using margins
> > when it's in terminal frames.
>
> Thanks, I think this is almost ready to be installed; see a few minor
> comments below.
>
> > Sadly I couldn't find anything for detecting HDiPI frames, so the user
> > must have to configure it.
>
> I don't think Emacs should second-guess the user in this matter: let
> the user decide whether the fringes are too narrow for these
> indicators.
>
> > +(defcustom flymake-indicator-type (if (display-graphic-p)
> > +                                      'fringes
> > +                                    'margins)
> > +  "Indicate which indicator type to use for display errors.
> > +
> > +The value can be nil (dont indicate errors but just highlight them),
>                          ^^^^
> "don't"
>
> > +(defcustom flymake-margin-indicators-string
> > +  '((error "!!" compilation-error)
> > +    (warning "!" compilation-warning)
> > +    (note "!" compilation-info))
> > +  "Strings used for margins indicators.
> > +The value of each list may be a list of 3 elements where specifies the
> > +error type, its string to use and its face,
>                ^^^^^^^^^^
> "the string"
>
> > +or a list of 2 elements specifying only the error type and its string.
>                                                           ^^^^^^^^^^^^^^
> "and the corresponding string"
>
> > +Indicators can be an ASCII or non-ASCII string.
>
> This sentence is redundant, so I suggest to remove it.  The ASCII vs
> non-ASCII issue is only relevant for the default value; users can do
> whatever they want when they customize the option.
>
> > +(defcustom flymake-autoresize-margins t
> > +  "If non-nil automatically resize margin-width.
>                 ^
> Comma is missing there.  Also, how about telling what triggers
> the resizing?
>
> > +Only if `flymake-indicator-type' is set to margins."
>    ^^^^^^^
> "Only relevant if..."
>
> > +(defvar-local flymake-original-margin-width nil
> > +  "Store original margin width")
>
> Should this be an internal variable (named flymake--original-margin-width)?
> Also, please consider telling in the doc string when is the width restored.
>
> > +(defun flymake--resize-margins (&optional og-width)
> > +  "Resize current window margins according `flymake-margin-indicator-position'.
>                                     ^^^^^^^^^
> "according to"
>
> > +Return to original margin width if OG-WIDTH is non-nil."
>
> Suggest to rename OG-WIDTH to ORIG-WIDTH.
>
> Last, but not least: please mention this change (and the new user
> options) in NEWS and in the Flymake manual.

Thank you, I've now fixed the patch and documented the changes done.

Also I have a question, Do I need to assign copyright each time
that I want to contribute to Flymake?

I have a lot of ideas that I would like to contribute.

[-- Attachment #2: 0001-Flymake-support-for-indicating-errors-in-margin.patch --]
[-- Type: application/octet-stream, Size: 13013 bytes --]

From 4ed73c51e7d82b6ade8ecb519066ba7e006beaf4 Mon Sep 17 00:00:00 2001
From: "Elias G. B. Perez" <eg642616@gmail.com>
Date: Sat, 6 Apr 2024 13:57:30 -0600
Subject: [PATCH] Flymake support for indicating errors in margin

Add optional support for display flymake error in margin,
this allow displaying error indicators in both graphical and
terminal frames.
* doc/misc/flymake.texi (Customizable variables)
(Flymake error types): Document new margin indicator.
* etc/NEWS: Announce the new Flymake user option for margin
indicators.
* lisp/progmodes/flymake.el (flymake-indicator-type)
(flymake-margin-indicators-string, flymake-autoresize-margins)
(flymake-margin-indicator-position): New user options.
(flymake--original-margin-width): Add buffer-local variable for
store original buffer margin width.
(flymake-error, flymake-warning, flymake-note): Use new margin
value.
(flymake--indicator-overlay-spec): Rework and Rename from
flymake--fringe-overlay-spec.
(flymake--resize-margins): Add new function for resize margin
width.
(flymake--highlight-line, flymake-mode): Rework.
---
 doc/misc/flymake.texi     |  25 +++++++
 etc/NEWS                  |  21 ++++++
 lisp/progmodes/flymake.el | 134 +++++++++++++++++++++++++++++++++-----
 3 files changed, 165 insertions(+), 15 deletions(-)

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
index 84a74a9..7b42de0 100644
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -309,6 +309,11 @@ Customizable variables
 A custom face for highlighting regions for which a note has been
 reported.
 
+@item flymake-indicator-type
+Which indicator type should Flymake use.
+Depending on your preference, this can either use @code{fringes} or
+@code{margins} for indicating errors.
+
 @item flymake-error-bitmap
 A bitmap used in the fringe to mark lines for which an error has
 been reported.
@@ -320,6 +325,18 @@ Customizable variables
 @item flymake-fringe-indicator-position
 Which fringe (if any) should show the warning/error bitmaps.
 
+@item flymake-margin-indicators-string
+A List used for specify string, face and error types to
+use in the margin indicators.
+
+@item flymake-margin-indicator-position
+Which margin (if any) should show the warning/error strings.
+
+@item flymake-autoresize-margins
+If non-@code{nil}, will resize margins either @code{flymake-mode} is
+non-@code{nil} or @code{nil}.
+Only relevant if @code{flymake-indicator-type} is set to @code{margins}.
+
 @item flymake-wrap-around
 If non-@code{nil}, moving to errors with @code{flymake-goto-next-error} and
 @code{flymake-goto-prev-error} wraps around buffer boundaries.
@@ -387,6 +404,14 @@ Flymake error types
 variables}).  It is overridden by any @code{before-string} overlay
 property.
 
+@item
+@cindex margin of diagnostic
+@code{flymake-margin-string}, an string displayed in the margin
+according to @code{flymake-margin-indicator-position}.
+The value actually follows the syntax of @code{flymake-margin-indicators-string}
+(@pxref{Customizable variables}). It is overridden by any
+@code{before-string} overlay property.
+
 @item
 @code{flymake-overlay-control}, an alist ((@var{OVPROP} . @var{VALUE})
 @var{...}) of further properties used to affect the appearance of
diff --git a/etc/NEWS b/etc/NEWS
index 375c27a..8a7ecfb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1201,6 +1201,27 @@ in a clean environment.
 
 ** Flymake
 
++++
+*** New user option 'flymake-indicator-type'.
+This user option controls which error indicator type Flymake should use
+in current buffer. Depending on your preference, this can either use
+fringes or margins for indicating errors.
+
++++
+*** New user option 'flymake-margin-indicators-string'
+It controls the string, their face and type error to display in
+margin indicators.
+
++++
+*** New user option 'flymake-autoresize-margins'
+When non-nil, Flymake will resize margins either `flymake-mode` is
+non-nil or nil.
+Only relevant if `flymake-indicator-type` is set to `margins`.
+
++++
+*** New user option 'flymake-margin-indicator-position'
+It controls which position should Flymake show margin indicators.
+
 +++
 *** 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 779c612..fc64368 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -180,6 +180,59 @@ flymake-fringe-indicator-position
 		 (const right-fringe)
 		 (const :tag "No fringe indicators" nil)))
 
+(defcustom flymake-indicator-type (if (display-graphic-p)
+                                      'fringes
+                                    'margins)
+  "Indicate which indicator type to use for display errors.
+
+The value can be nil (don't 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
+from current 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)))
+
+(defcustom flymake-margin-indicators-string
+  '((error "!!" compilation-error)
+    (warning "!" compilation-warning)
+    (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, the string to use and its face,
+or a list of 2 elements specifying only the error type and
+the corresponding string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(defcustom flymake-autoresize-margins t
+  "If non-nil, automatically resize margin-width calling flymake--resize-margins.
+
+Only relevant if `flymake-indicator-type' is set to margins."
+  :version "30.1"
+  :type 'boolean)
+
+(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-margin-indicators-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")
@@ -258,6 +311,11 @@ flymake-timer
 (defvar-local flymake-check-start-time nil
   "Time at which syntax check was started.")
 
+(defvar-local flymake--original-margin-width nil
+  "Store original margin width.
+Used by `flymake--resize-margins' for restore original margin width
+when flymake is turned off.")
+
 (defun flymake--log-1 (level sublog msg &rest args)
   "Do actual work for `flymake-log'."
   (let (;; never popup the log buffer
@@ -630,6 +688,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-error 'face 'flymake-error)
 (put 'flymake-error 'flymake-bitmap 'flymake-error-bitmap)
+(put 'flymake-error 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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 +697,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-warning 'face 'flymake-warning)
 (put 'flymake-warning 'flymake-bitmap 'flymake-warning-bitmap)
+(put 'flymake-warning 'flymake-margin-string (alist-get 'warning flymake-margin-indicators-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 +706,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-note 'face 'flymake-note)
 (put 'flymake-note 'flymake-bitmap 'flymake-note-bitmap)
+(put 'flymake-note 'flymake-margin-string (alist-get 'note flymake-margin-indicators-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 +743,53 @@ flymake--severity
   (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--indicator-overlay-spec (indicator)
+  "Return INDICATOR 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)))))))))
+
+(defun flymake--resize-margins (&optional orig-width)
+  "Resize current window margins according to `flymake-margin-indicator-position'.
+Return to original margin width if ORIG-WIDTH is non-nil."
+  (when (and (eq flymake-indicator-type 'margins)
+             flymake-autoresize-margins)
+    (cond
+     ((and orig-width flymake--original-margin-width)
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local left-margin-width flymake--original-margin-width)
+        (setq-local right-margin-width flymake--original-margin-width)))
+     (t
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local flymake--original-margin-width left-margin-width
+                      left-margin-width 2)
+        (setq-local flymake--original-margin-width right-margin-width
+                    right-margin-width 2))))
+    ;; Apply margin to all windows avalaibles
+    (mapc (lambda (x)
+            (set-window-buffer x (window-buffer x)))
+          (get-buffer-window-list nil nil 'visible))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -840,10 +935,13 @@ flymake--highlight-line
                                         type prop value)))))
       (default-maybe 'face 'flymake-error)
       (default-maybe 'before-string
-        (flymake--fringe-overlay-spec
+        (flymake--indicator-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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
@@ -1285,6 +1383,9 @@ flymake-mode
     (add-hook 'kill-buffer-hook 'flymake-kill-buffer-hook nil t)
     (add-hook 'eldoc-documentation-functions 'flymake-eldoc-function t t)
 
+    ;; AutoResize margins.
+    (flymake--resize-margins)
+
     ;; If Flymake happened to be already ON, we must cleanup
     ;; existing diagnostic overlays, lest we forget them by blindly
     ;; reinitializing `flymake--state' in the next line.
@@ -1333,6 +1434,9 @@ flymake-mode
     ;;+(remove-hook 'find-file-hook (function flymake-find-file-hook) t)
     (remove-hook 'eldoc-documentation-functions 'flymake-eldoc-function t)
 
+    ;; return margin to original size
+    (flymake--resize-margins t)
+
     (when flymake-timer
       (cancel-timer flymake-timer)
       (setq flymake-timer nil))
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-04-06 20:14                               ` Elijah G
@ 2024-04-07  5:47                                 ` Eli Zaretskii
  2024-04-07 17:20                                   ` Elijah G
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2024-04-07  5:47 UTC (permalink / raw)
  To: Elijah G; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

> From: Elijah G <eg642616@gmail.com>
> Date: Sat, 6 Apr 2024 14:14:34 -0600
> Cc: sbaugh@catern.com, sbaugh@janestreet.com, philipk@posteo.net, 
> 	emacs-devel@gnu.org, luangruo@yahoo.com
> 
> > Last, but not least: please mention this change (and the new user
> > options) in NEWS and in the Flymake manual.
> 
> Thank you, I've now fixed the patch and documented the changes done.

See below for some additional comments.

> Also I have a question, Do I need to assign copyright each time
> that I want to contribute to Flymake?

No, you assign the copyright for past and future changes, and you do
it only once.

Did you start the paperwork already?  I don't see your assignment on
file, so if you haven't started the paperwork, now is the time to do
it.  Let me know if you need me to send you the form to fill and the
instructions, to start the paperwork.

> I have a lot of ideas that I would like to contribute.

Thanks, looking forward to your contributions.

> --- a/doc/misc/flymake.texi
> +++ b/doc/misc/flymake.texi
> @@ -309,6 +309,11 @@ Customizable variables
>  A custom face for highlighting regions for which a note has been
>  reported.
>  
> +@item flymake-indicator-type
> +Which indicator type should Flymake use.

Suggest to reword:

  The indicator type which Flymake should use to indicate lines with
  errors or warnings.

> +@item flymake-margin-indicators-string
> +A List used for specify string, face and error types to
> +use in the margin indicators.

Suggest to reword:

  Specifies the string and face to use for the margin indicators, for
  each error type.

> +@item flymake-autoresize-margins
> +If non-@code{nil}, will resize margins either @code{flymake-mode} is
> +non-@code{nil} or @code{nil}.

Something is wrong with this sentence.  You probably mean

   If non-@code{nil}, Flymake will resize the margins when
   @code{flymake-mode} is turned on or off.

> +@item
> +@cindex margin of diagnostic
> +@code{flymake-margin-string}, an string displayed in the margin
                                 ^^^^^^^^^
"a string"

> +The value actually follows the syntax of @code{flymake-margin-indicators-string}
> +(@pxref{Customizable variables}). It is overridden by any
                                   ^^
Two spaces between sentences, please.

> ++++
> +*** New user option 'flymake-indicator-type'.
> +This user option controls which error indicator type Flymake should use
> +in current buffer. Depending on your preference, this can either use
                    ^^
Likewise.

> +*** New user option 'flymake-margin-indicators-string'

All heading lines in NEWS should end in a period.

> +It controls the string, their face and type error to display in
> +margin indicators.

  It controls, for each error type, the string and its face to display
  as the margin indicator.

> +*** New user option 'flymake-autoresize-margins'

Missing period at the end.

> +When non-nil, Flymake will resize margins either `flymake-mode` is
> +non-nil or nil.

See above for correction in the manual, I think the same wording
should be used here.

Also, please quote 'like this', not `like this` (here and elsewhere in
the patch for NEWS).

> +*** New user option 'flymake-margin-indicator-position'

Period missing at the end.

> +It controls which position should Flymake show margin indicators.

  It controls which margin (left or right) is used for margin
  indicators.

> +(defvar-local flymake--original-margin-width nil
> +  "Store original margin width.
> +Used by `flymake--resize-margins' for restore original margin width
                                     ^^^^^^^^^^^
"for restoring"



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-04-07  5:47                                 ` Eli Zaretskii
@ 2024-04-07 17:20                                   ` Elijah G
  2024-04-18  9:10                                     ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Elijah G @ 2024-04-07 17:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

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

On Sat, Apr 6, 2024 at 11:48 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Elijah G <eg642616@gmail.com>
> > Date: Sat, 6 Apr 2024 14:14:34 -0600
> > Cc: sbaugh@catern.com, sbaugh@janestreet.com, philipk@posteo.net,
> >       emacs-devel@gnu.org, luangruo@yahoo.com
> >
> > > Last, but not least: please mention this change (and the new user
> > > options) in NEWS and in the Flymake manual.
> >
> > Thank you, I've now fixed the patch and documented the changes done.
>
> See below for some additional comments.

Thank you, I've now fixed the patch.
Sorry if this is the second time I fixed the patch,
I'm not good at documenting code.

> > Also I have a question, Do I need to assign copyright each time
> > that I want to contribute to Flymake?
>
> No, you assign the copyright for past and future changes, and you do
> it only once.
>
> Did you start the paperwork already?  I don't see your assignment on
> file, so if you haven't started the paperwork, now is the time to do
> it.  Let me know if you need me to send you the form to fill and the
> instructions, to start the paperwork.

Thanks, I've already signed the paperwork and sent to assign@gnu.org,
however i didn't get any response, i'm not sure if i wrote the subject well
in the email, Can you help me how I should send my signed paperwork
off-list?

Thanks.

[-- Attachment #2: 0001-Flymake-support-for-indicating-errors-in-margin.patch --]
[-- Type: application/octet-stream, Size: 13085 bytes --]

From ddc8aeb35b05fba0b9feaccab3f95c96cf49213e Mon Sep 17 00:00:00 2001
From: "Elias G. B. Perez" <eg642616@gmail.com>
Date: Sat, 6 Apr 2024 13:57:30 -0600
Subject: [PATCH] Flymake support for indicating errors in margin

Add optional support for display flymake error in margin,
this allow displaying error indicators in both graphical and
terminal frames.
* doc/misc/flymake.texi (Customizable variables)
(Flymake error types): Document new margin indicator.
* etc/NEWS: Announce the new Flymake user option for margin
indicators.
* lisp/progmodes/flymake.el (flymake-indicator-type)
(flymake-margin-indicators-string, flymake-autoresize-margins)
(flymake-margin-indicator-position): New user options.
(flymake--original-margin-width): Add buffer-local variable for
store original buffer margin width.
(flymake-error, flymake-warning, flymake-note): Use new margin
value.
(flymake--indicator-overlay-spec): Rework and Rename from
flymake--fringe-overlay-spec.
(flymake--resize-margins): Add new function for resize margin
width.
(flymake--highlight-line, flymake-mode): Rework.
---
 doc/misc/flymake.texi     |  26 ++++++++
 etc/NEWS                  |  22 +++++++
 lisp/progmodes/flymake.el | 134 +++++++++++++++++++++++++++++++++-----
 3 files changed, 167 insertions(+), 15 deletions(-)

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
index 84a74a9..7019f4b 100644
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -309,6 +309,12 @@ Customizable variables
 A custom face for highlighting regions for which a note has been
 reported.
 
+@item flymake-indicator-type
+The indicator type which Flymake should use to indicate lines with
+errors or warnings.
+Depending on your preference, this can either use @code{fringes} or
+@code{margins} for indicating errors.
+
 @item flymake-error-bitmap
 A bitmap used in the fringe to mark lines for which an error has
 been reported.
@@ -320,6 +326,18 @@ Customizable variables
 @item flymake-fringe-indicator-position
 Which fringe (if any) should show the warning/error bitmaps.
 
+@item flymake-margin-indicators-string
+Specifies the string and face to use for the margin indicators, for
+each error type.
+
+@item flymake-margin-indicator-position
+Which margin (if any) should show the warning/error strings.
+
+@item flymake-autoresize-margins
+If non-@code{nil}, Flymake will resize the margins when
+@code{flymake-mode} is turned on or off.
+Only relevant if @code{flymake-indicator-type} is set to @code{margins}.
+
 @item flymake-wrap-around
 If non-@code{nil}, moving to errors with @code{flymake-goto-next-error} and
 @code{flymake-goto-prev-error} wraps around buffer boundaries.
@@ -387,6 +405,14 @@ Flymake error types
 variables}).  It is overridden by any @code{before-string} overlay
 property.
 
+@item
+@cindex margin of diagnostic
+@code{flymake-margin-string}, a string displayed in the margin
+according to @code{flymake-margin-indicator-position}.
+The value actually follows the syntax of @code{flymake-margin-indicators-string}
+(@pxref{Customizable variables}).  It is overridden by any
+@code{before-string} overlay property.
+
 @item
 @code{flymake-overlay-control}, an alist ((@var{OVPROP} . @var{VALUE})
 @var{...}) of further properties used to affect the appearance of
diff --git a/etc/NEWS b/etc/NEWS
index 375c27a..6e86959 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1201,6 +1201,28 @@ in a clean environment.
 
 ** Flymake
 
++++
+*** New user option 'flymake-indicator-type'.
+This user option controls which error indicator type Flymake should use
+in current buffer.  Depending on your preference, this can either use
+fringes or margins for indicating errors.
+
++++
+*** New user option 'flymake-margin-indicators-string'.
+It controls, for each error type, the string and its face to display as
+the margin indicator.
+
++++
+*** New user option 'flymake-autoresize-margins'.
+If non-nil, Flymake will resize the margins when 'flymake-mode' is
+turned on or off.
+Only relevant if `flymake-indicator-type` is set to `margins`.
+
++++
+*** New user option 'flymake-margin-indicator-position'.
+It controls which margin (left or right) is used for margin
+indicators.
+
 +++
 *** 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 779c612..b5b9988 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -180,6 +180,59 @@ flymake-fringe-indicator-position
 		 (const right-fringe)
 		 (const :tag "No fringe indicators" nil)))
 
+(defcustom flymake-indicator-type (if (display-graphic-p)
+                                      'fringes
+                                    'margins)
+  "Indicate which indicator type to use for display errors.
+
+The value can be nil (don't 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
+from current 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)))
+
+(defcustom flymake-margin-indicators-string
+  '((error "!!" compilation-error)
+    (warning "!" compilation-warning)
+    (note "!" compilation-info))
+  "Strings used for margins indicators.
+The value of each list may be a list of 3 elements where specifies the
+error type, the string to use and its face,
+or a list of 2 elements specifying only the error type and
+the corresponding string.
+
+The option `flymake-margin-indicator-position' controls how and where
+this is used."
+  :version "30.1"
+  :type '(repeat :tag "Error types lists"
+                 (list :tag "String and face for error types"
+                       (symbol :tag "Error type")
+                       (string :tag "String")
+                       (face :tag "Face"))))
+
+(defcustom flymake-autoresize-margins t
+  "If non-nil, automatically resize margin-width calling flymake--resize-margins.
+
+Only relevant if `flymake-indicator-type' is set to margins."
+  :version "30.1"
+  :type 'boolean)
+
+(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-margin-indicators-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")
@@ -258,6 +311,11 @@ flymake-timer
 (defvar-local flymake-check-start-time nil
   "Time at which syntax check was started.")
 
+(defvar-local flymake--original-margin-width nil
+  "Store original margin width.
+Used by `flymake--resize-margins' for restoring original margin width
+when flymake is turned off.")
+
 (defun flymake--log-1 (level sublog msg &rest args)
   "Do actual work for `flymake-log'."
   (let (;; never popup the log buffer
@@ -630,6 +688,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-error 'face 'flymake-error)
 (put 'flymake-error 'flymake-bitmap 'flymake-error-bitmap)
+(put 'flymake-error 'flymake-margin-string (alist-get 'error flymake-margin-indicators-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 +697,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-warning 'face 'flymake-warning)
 (put 'flymake-warning 'flymake-bitmap 'flymake-warning-bitmap)
+(put 'flymake-warning 'flymake-margin-string (alist-get 'warning flymake-margin-indicators-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 +706,7 @@ flymake-diagnostic-types-alist
 
 (put 'flymake-note 'face 'flymake-note)
 (put 'flymake-note 'flymake-bitmap 'flymake-note-bitmap)
+(put 'flymake-note 'flymake-margin-string (alist-get 'note flymake-margin-indicators-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 +743,53 @@ flymake--severity
   (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--indicator-overlay-spec (indicator)
+  "Return INDICATOR 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)))))))))
+
+(defun flymake--resize-margins (&optional orig-width)
+  "Resize current window margins according to `flymake-margin-indicator-position'.
+Return to original margin width if ORIG-WIDTH is non-nil."
+  (when (and (eq flymake-indicator-type 'margins)
+             flymake-autoresize-margins)
+    (cond
+     ((and orig-width flymake--original-margin-width)
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local left-margin-width flymake--original-margin-width)
+        (setq-local right-margin-width flymake--original-margin-width)))
+     (t
+      (if (eq flymake-margin-indicator-position 'left-margin)
+          (setq-local flymake--original-margin-width left-margin-width
+                      left-margin-width 2)
+        (setq-local flymake--original-margin-width right-margin-width
+                    right-margin-width 2))))
+    ;; Apply margin to all windows avalaibles
+    (mapc (lambda (x)
+            (set-window-buffer x (window-buffer x)))
+          (get-buffer-window-list nil nil 'visible))))
 
 (defun flymake--equal-diagnostic-p (a b)
   "Tell if A and B are equivalent `flymake--diag' objects."
@@ -840,10 +935,13 @@ flymake--highlight-line
                                         type prop value)))))
       (default-maybe 'face 'flymake-error)
       (default-maybe 'before-string
-        (flymake--fringe-overlay-spec
+        (flymake--indicator-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))
           (alist-get 'bitmap (alist-get type ; backward compat
                                         flymake-diagnostic-types-alist)))))
       ;; (default-maybe 'after-string
@@ -1285,6 +1383,9 @@ flymake-mode
     (add-hook 'kill-buffer-hook 'flymake-kill-buffer-hook nil t)
     (add-hook 'eldoc-documentation-functions 'flymake-eldoc-function t t)
 
+    ;; AutoResize margins.
+    (flymake--resize-margins)
+
     ;; If Flymake happened to be already ON, we must cleanup
     ;; existing diagnostic overlays, lest we forget them by blindly
     ;; reinitializing `flymake--state' in the next line.
@@ -1333,6 +1434,9 @@ flymake-mode
     ;;+(remove-hook 'find-file-hook (function flymake-find-file-hook) t)
     (remove-hook 'eldoc-documentation-functions 'flymake-eldoc-function t)
 
+    ;; return margin to original size
+    (flymake--resize-margins t)
+
     (when flymake-timer
       (cancel-timer flymake-timer)
       (setq flymake-timer nil))
-- 
2.44.0.windows.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Flymake Support Indicator Errors in Margin
  2024-04-07 17:20                                   ` Elijah G
@ 2024-04-18  9:10                                     ` Eli Zaretskii
  0 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2024-04-18  9:10 UTC (permalink / raw)
  To: Elijah G; +Cc: sbaugh, sbaugh, philipk, emacs-devel, luangruo

> From: Elijah G <eg642616@gmail.com>
> Date: Sun, 7 Apr 2024 11:20:41 -0600
> Cc: sbaugh@catern.com, sbaugh@janestreet.com, philipk@posteo.net, 
> 	emacs-devel@gnu.org, luangruo@yahoo.com
> 
> On Sat, Apr 6, 2024 at 11:48 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Elijah G <eg642616@gmail.com>
> > > Date: Sat, 6 Apr 2024 14:14:34 -0600
> > > Cc: sbaugh@catern.com, sbaugh@janestreet.com, philipk@posteo.net,
> > >       emacs-devel@gnu.org, luangruo@yahoo.com
> > >
> > > > Last, but not least: please mention this change (and the new user
> > > > options) in NEWS and in the Flymake manual.
> > >
> > > Thank you, I've now fixed the patch and documented the changes done.
> >
> > See below for some additional comments.
> 
> Thank you, I've now fixed the patch.
> Sorry if this is the second time I fixed the patch,
> I'm not good at documenting code.
> 
> > > Also I have a question, Do I need to assign copyright each time
> > > that I want to contribute to Flymake?
> >
> > No, you assign the copyright for past and future changes, and you do
> > it only once.
> >
> > Did you start the paperwork already?  I don't see your assignment on
> > file, so if you haven't started the paperwork, now is the time to do
> > it.  Let me know if you need me to send you the form to fill and the
> > instructions, to start the paperwork.
> 
> Thanks, I've already signed the paperwork and sent to assign@gnu.org,
> however i didn't get any response, i'm not sure if i wrote the subject well
> in the email, Can you help me how I should send my signed paperwork
> off-list?
> 
> Thanks.

Thank you, I have now installed this on the master branch.



^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2024-04-18  9:10 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-11 23:18 [PATCH] Flymake Support Indicator Errors in Margin Elijah G
2024-03-12  9:24 ` Philip Kaludercic
2024-03-12 17:22   ` Elijah G
2024-03-13 12:18     ` Eli Zaretskii
2024-03-14  1:50       ` Elijah G
2024-03-14 11:05         ` Eli Zaretskii
2024-03-14 11:28           ` João Távora
2024-03-14 15:35           ` Elijah G
2024-03-16 11:10             ` Eli Zaretskii
2024-03-17 16:44               ` bird
2024-03-17 17:01                 ` Eli Zaretskii
2024-03-17 17:34                 ` Elijah G
2024-03-17 18:43                   ` bird
2024-03-17 19:21                     ` Elijah G
2024-03-25  1:46                       ` Elijah G
2024-03-27  0:13                         ` sbaugh
2024-03-27  0:36                           ` Elijah G
2024-03-27 21:29                           ` Elijah G
2024-03-28  6:01                             ` Eli Zaretskii
2024-03-28 17:34                               ` Elijah G
2024-04-06 11:36                                 ` Eli Zaretskii
2024-03-28  7:30                             ` Juri Linkov
2024-03-28 17:44                               ` Elijah G
2024-04-06 11:35                             ` Eli Zaretskii
2024-04-06 20:14                               ` Elijah G
2024-04-07  5:47                                 ` Eli Zaretskii
2024-04-07 17:20                                   ` Elijah G
2024-04-18  9:10                                     ` Eli Zaretskii
2024-03-19  7:03                   ` Augusto Stoffel
2024-03-17 17:49                 ` Elijah G
2024-03-19  7:04                   ` Augusto Stoffel

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