unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
@ 2019-07-30 20:44 Juri Linkov
  2019-07-30 21:21 ` Juri Linkov
  2019-07-31  2:30 ` Eli Zaretskii
  0 siblings, 2 replies; 16+ messages in thread
From: Juri Linkov @ 2019-07-30 20:44 UTC (permalink / raw)
  To: 36861

It has an unpleasant effect when git commit abruptly fails when
the length of the first line of the commit message is longer than
78 characters.  This patch enables a visual indication in the
log-edit buffer to help not to exceed the limit beforehand:

diff --git a/.dir-locals.el b/.dir-locals.el
index ffd65c8802..e56d8c753b 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -10,7 +10,9 @@
                (electric-quote-comment . nil)
                (electric-quote-string . nil)))
  (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
-                   (log-edit-setup-add-author . t)))
+                   (log-edit-setup-add-author . t)
+                   (fill-column . 78)
+                   (eval . (display-fill-column-indicator-mode))))
  (change-log-mode . ((add-log-time-zone-rule . t)
 		     (fill-column . 74)
 		     (eval . (bug-reference-mode))))





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-07-30 20:44 bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode Juri Linkov
@ 2019-07-30 21:21 ` Juri Linkov
  2019-07-31  2:30 ` Eli Zaretskii
  1 sibling, 0 replies; 16+ messages in thread
From: Juri Linkov @ 2019-07-30 21:21 UTC (permalink / raw)
  To: 36861

> +                   (fill-column . 78)

Sorry, I missed the special variable introduced by this feature -
‘display-fill-column-indicator-column’.  But ‘fill-column’ should not be
changed, it should remain at its default value.  Here's the correct patch:

diff --git a/.dir-locals.el b/.dir-locals.el
index ffd65c8802..ad5eb066ca 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -10,7 +10,9 @@
                (electric-quote-comment . nil)
                (electric-quote-string . nil)))
  (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
-                   (log-edit-setup-add-author . t)))
+                   (log-edit-setup-add-author . t)
+                   (display-fill-column-indicator-column . 78)
+                   (eval . (display-fill-column-indicator-mode))))
  (change-log-mode . ((add-log-time-zone-rule . t)
 		     (fill-column . 74)
 		     (eval . (bug-reference-mode))))





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-07-30 20:44 bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode Juri Linkov
  2019-07-30 21:21 ` Juri Linkov
@ 2019-07-31  2:30 ` Eli Zaretskii
  2019-07-31 20:49   ` Juri Linkov
  1 sibling, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-07-31  2:30 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 36861

> From: Juri Linkov <juri@linkov.net>
> Date: Tue, 30 Jul 2019 23:44:53 +0300
> 
> It has an unpleasant effect when git commit abruptly fails when
> the length of the first line of the commit message is longer than
> 78 characters.  This patch enables a visual indication in the
> log-edit buffer to help not to exceed the limit beforehand:
> 
> diff --git a/.dir-locals.el b/.dir-locals.el
> index ffd65c8802..e56d8c753b 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -10,7 +10,9 @@
>                 (electric-quote-comment . nil)
>                 (electric-quote-string . nil)))
>   (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
> -                   (log-edit-setup-add-author . t)))
> +                   (log-edit-setup-add-author . t)
> +                   (fill-column . 78)
> +                   (eval . (display-fill-column-indicator-mode))))

This will cause an annoying message and prompt when editing Emacs
sources with an Emacs which doesn't yet have
display-fill-column-indicator-mode, right?  Can we avoid that?  I
routinely need to work on the latest sources with an older Emacs.





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-07-31  2:30 ` Eli Zaretskii
@ 2019-07-31 20:49   ` Juri Linkov
  2019-08-02  9:10     ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2019-07-31 20:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 36861

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

>>   (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
>> -                   (log-edit-setup-add-author . t)))
>> +                   (log-edit-setup-add-author . t)
>> +                   (display-fill-column-indicator-column . 78)
>> +                   (eval . (display-fill-column-indicator-mode))))
>
> This will cause an annoying message and prompt when editing Emacs
> sources with an Emacs which doesn't yet have
> display-fill-column-indicator-mode, right?  Can we avoid that?  I
> routinely need to work on the latest sources with an older Emacs.

Shouldn't local-variables functions ignore undefined variables and commands?
Probably not, since such change won't help for older versions.

Then one way is to put such lines to the init file
to avoid typing `y' to confirm local variables
while using emacs-26 to commit emacs-27 changes:

  (put 'display-fill-column-indicator 'safe-local-variable 'booleanp)
  (put 'display-fill-column-indicator-character 'safe-local-variable 'characterp)
  (put 'display-fill-column-indicator-column 'safe-local-variable
       (lambda (value) (or (booleanp value) (integerp value))))
  (defun display-fill-column-indicator-mode ())

And for emacs-27 and future versions this patch is required as well:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: display-fill-column-indicator-cus-start.patch --]
[-- Type: text/x-diff, Size: 1276 bytes --]

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index e1d0bce2ad..036674ef14 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -650,7 +650,7 @@ minibuffer-prompt-properties--setter
                                  "26.1")
 
              (display-fill-column-indicator display-fill-column-indicator
-                                 boolean "27.1")
+                                 boolean "27.1" :safe booleanp)
              (display-fill-column-indicator-column display-fill-column-indicator
                                  (choice
                                   (const :tag "Use fill-column variable"
@@ -659,9 +659,9 @@ minibuffer-prompt-properties--setter
                                          :value 70
                                          :format "%v")
                                   integer)
-                                 "27.1")
+                                 "27.1" :safe (lambda (value) (or (booleanp value) (integerp value))))
              (display-fill-column-indicator-character display-fill-column-indicator
-                                 character "27.1")
+                                 character "27.1" :safe characterp)
 	     ;; xfaces.c
 	     (scalable-fonts-allowed display boolean "22.1")
 	     ;; xfns.c

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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-07-31 20:49   ` Juri Linkov
@ 2019-08-02  9:10     ` Eli Zaretskii
  2019-08-03 22:31       ` Juri Linkov
  2019-08-05 21:43       ` Juri Linkov
  0 siblings, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2019-08-02  9:10 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 36861

> From: Juri Linkov <juri@linkov.net>
> Cc: 36861@debbugs.gnu.org
> Date: Wed, 31 Jul 2019 23:49:53 +0300
> 
> >>   (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
> >> -                   (log-edit-setup-add-author . t)))
> >> +                   (log-edit-setup-add-author . t)
> >> +                   (display-fill-column-indicator-column . 78)
> >> +                   (eval . (display-fill-column-indicator-mode))))
> >
> > This will cause an annoying message and prompt when editing Emacs
> > sources with an Emacs which doesn't yet have
> > display-fill-column-indicator-mode, right?  Can we avoid that?  I
> > routinely need to work on the latest sources with an older Emacs.
> 
> Shouldn't local-variables functions ignore undefined variables and commands?
> Probably not, since such change won't help for older versions.

Right, and we cannot summarily allow any variables a given Emacs
doesn't know about, that'd we unsafe.

> Then one way is to put such lines to the init file
> to avoid typing `y' to confirm local variables
> while using emacs-26 to commit emacs-27 changes:

Rather than requiring users of older Emacsen to change their init
files in such strange ways, which will/might be a problem when they
upgrade to Emacs 27, why not expect users who want the early detection
of long lines to turn on display-fill-column-indicator-mode in their
init files?  IOW, the solution that requires changes to one's init
files goes both ways.

> And for emacs-27 and future versions this patch is required as well:
> 
> diff --git a/lisp/cus-start.el b/lisp/cus-start.el
> index e1d0bce2ad..036674ef14 100644
> --- a/lisp/cus-start.el
> +++ b/lisp/cus-start.el
> @@ -650,7 +650,7 @@ minibuffer-prompt-properties--setter
>                                   "26.1")
>  
>               (display-fill-column-indicator display-fill-column-indicator
> -                                 boolean "27.1")
> +                                 boolean "27.1" :safe booleanp)
>               (display-fill-column-indicator-column display-fill-column-indicator
>                                   (choice
>                                    (const :tag "Use fill-column variable"
> @@ -659,9 +659,9 @@ minibuffer-prompt-properties--setter
>                                           :value 70
>                                           :format "%v")
>                                    integer)
> -                                 "27.1")
> +                                 "27.1" :safe (lambda (value) (or (booleanp value) (integerp value))))
>               (display-fill-column-indicator-character display-fill-column-indicator
> -                                 character "27.1")
> +                                 character "27.1" :safe characterp)
>  	     ;; xfaces.c
>  	     (scalable-fonts-allowed display boolean "22.1")
>  	     ;; xfns.c

This is fine with me, but I don't think we should make the change in
.dir-locals.el.

Thanks.





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-02  9:10     ` Eli Zaretskii
@ 2019-08-03 22:31       ` Juri Linkov
  2019-08-04  0:51         ` Ergus
  2019-08-05 21:43       ` Juri Linkov
  1 sibling, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2019-08-03 22:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: spacibba, 36861

>> >>   (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
>> >> -                   (log-edit-setup-add-author . t)))
>> >> +                   (log-edit-setup-add-author . t)
>> >> +                   (display-fill-column-indicator-column . 78)
>> >> +                   (eval . (display-fill-column-indicator-mode))))
>> >
>> > This will cause an annoying message and prompt when editing Emacs
>> > sources with an Emacs which doesn't yet have
>> > display-fill-column-indicator-mode, right?  Can we avoid that?  I
>> > routinely need to work on the latest sources with an older Emacs.
>> 
>> Shouldn't local-variables functions ignore undefined variables and commands?
>> Probably not, since such change won't help for older versions.
>
> Right, and we cannot summarily allow any variables a given Emacs
> doesn't know about, that'd we unsafe.

Actually by using the word "ignore" I meant to not set an unbound variable
(currently `hack-local-variables' defines and sets unbound variables).
But this is not backward-compatible change.

>> Then one way is to put such lines to the init file
>> to avoid typing `y' to confirm local variables
>> while using emacs-26 to commit emacs-27 changes:
>
> Rather than requiring users of older Emacsen to change their init
> files in such strange ways, which will/might be a problem when they
> upgrade to Emacs 27, why not expect users who want the early detection
> of long lines to turn on display-fill-column-indicator-mode in their
> init files?  IOW, the solution that requires changes to one's init
> files goes both ways.

I don't understand why display-fill-column-indicator customization
should be more complicated than necessary?  Why it requires adding
these lines:

                   (display-fill-column-indicator-column . 78)
                   (eval . (display-fill-column-indicator-mode))

when it should be enough to avoid eval by:

                   (display-fill-column-indicator-column . 78)
                   (display-fill-column-indicator . t)

Is it because the only purpose of display-fill-column-indicator-mode
is to set display-fill-column-indicator-character?  But then why
display-fill-column-indicator-character is not nil by default?
This contradicts the docstring of display-fill-column-indicator-character
that says that the default is U+2502, but actually the default is nil.

I'm CC-ing the author of display-fill-column-indicator:
Could you please consider setting the default value of
display-fill-column-indicator-character?  This could
simplify customization.





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-03 22:31       ` Juri Linkov
@ 2019-08-04  0:51         ` Ergus
  2019-08-04 19:39           ` Juri Linkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ergus @ 2019-08-04  0:51 UTC (permalink / raw)
  To: Juri Linkov, Eli Zaretskii; +Cc: 36861

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

Hi, sorry, I don't understand actually why is so complex this provided code in the email. The initialization for display-fill-column-indicator makes some checks to set the default character as described in the documentation, so no extra code is needed for that in the user side.
In the initialization I see in this mail, they just set the column's value to 78 which can be done also using the variable fill-column for the whole major mode too. And actually dfci will recognize it by default and other functionalities too so in the general scenario is better to use that one.

(setq fill-column 78)
(display-fill-column-indicator t)


Should work no matters the order. Maybe as you were setting the mode's variable instead of calling the function with the same name; the mode was not properly initialized. 



On August 4, 2019 12:31:59 AM GMT+02:00, Juri Linkov <juri@linkov.net> wrote:
>>> >>   (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
>>> >> -                   (log-edit-setup-add-author . t)))
>>> >> +                   (log-edit-setup-add-author . t)
>>> >> +                   (display-fill-column-indicator-column . 78)
>>> >> +                   (eval .
>(display-fill-column-indicator-mode))))
>>> >
>>> > This will cause an annoying message and prompt when editing Emacs
>>> > sources with an Emacs which doesn't yet have
>>> > display-fill-column-indicator-mode, right?  Can we avoid that?  I
>>> > routinely need to work on the latest sources with an older Emacs.
>>> 
>>> Shouldn't local-variables functions ignore undefined variables and
>commands?
>>> Probably not, since such change won't help for older versions.
>>
>> Right, and we cannot summarily allow any variables a given Emacs
>> doesn't know about, that'd we unsafe.
>
>Actually by using the word "ignore" I meant to not set an unbound
>variable
>(currently `hack-local-variables' defines and sets unbound variables).
>But this is not backward-compatible change.
>
>>> Then one way is to put such lines to the init file
>>> to avoid typing `y' to confirm local variables
>>> while using emacs-26 to commit emacs-27 changes:
>>
>> Rather than requiring users of older Emacsen to change their init
>> files in such strange ways, which will/might be a problem when they
>> upgrade to Emacs 27, why not expect users who want the early
>detection
>> of long lines to turn on display-fill-column-indicator-mode in their
>> init files?  IOW, the solution that requires changes to one's init
>> files goes both ways.
>
>I don't understand why display-fill-column-indicator customization
>should be more complicated than necessary?  Why it requires adding
>these lines:
>
>                   (display-fill-column-indicator-column . 78)
>                   (eval . (display-fill-column-indicator-mode))
>
>when it should be enough to avoid eval by:
>
>                   (display-fill-column-indicator-column . 78)
>                   (display-fill-column-indicator . t)
>
>Is it because the only purpose of display-fill-column-indicator-mode
>is to set display-fill-column-indicator-character?  But then why
>display-fill-column-indicator-character is not nil by default?
>This contradicts the docstring of
>display-fill-column-indicator-character
>that says that the default is U+2502, but actually the default is nil.
>
>I'm CC-ing the author of display-fill-column-indicator:
>Could you please consider setting the default value of
>display-fill-column-indicator-character?  This could
>simplify customization.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-04  0:51         ` Ergus
@ 2019-08-04 19:39           ` Juri Linkov
  2019-08-04 20:30             ` Ergus
  0 siblings, 1 reply; 16+ messages in thread
From: Juri Linkov @ 2019-08-04 19:39 UTC (permalink / raw)
  To: Ergus; +Cc: 36861

> Hi, sorry, I don't understand actually why is so complex this provided code
> in the email. The initialization for display-fill-column-indicator makes
> some checks to set the default character as described in the documentation,
> so no extra code is needed for that in the user side.
> In the initialization I see in this mail, they just set the column's value
> to 78 which can be done also using the variable fill-column for the whole
> major mode too. And actually dfci will recognize it by default and other
> functionalities too so in the general scenario is better to use that one.
>
> (setq fill-column 78)
> (display-fill-column-indicator t)
>
> Should work no matters the order. Maybe as you were setting the mode's
> variable instead of calling the function with the same name; the mode
> was not properly initialized.

The problem is that is when the need is to enable dfci, it is simpler
to avoid eval to enable this mode because dfci works fine without using
dfci-mode, i.e. when only variables are set in Local Variables:

  ;;; Local Variables:
  ;;; display-fill-column-indicator: t
  ;;; display-fill-column-indicator-column: 78
  ;;; End:

or in .dir-locals.el:

  (display-fill-column-indicator . t)
  (display-fill-column-indicator-column . 78)

But the display-fill-column-indicator-character can't be set
in Local Variables because its value depends on the current display:
either U+2502 or ?| if the font does not support Unicode characters.

Do you think it would be possible to set the default value of
display-fill-column-indicator-character without calling
display-fill-column-indicator-mode?





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-04 19:39           ` Juri Linkov
@ 2019-08-04 20:30             ` Ergus
  2019-08-06 14:59               ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Ergus @ 2019-08-04 20:30 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 36861

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

Dfci was not designed to be used in this way, and the mode initialization actually sets the character, but in the future it could make many other dynamic tests/checks if needed. So I don't think we should modify it to work the other way you suggest, because for menit is not general enough. But I will follow the Eli's suggestions in this aspects more than my own opinion. 
Any way, if you want to use the mode in this way (for now) you can/need to set the display-fill-column-indicator-character to your desired value and it should work as it is now. But I can't promise that it will work the same way in the future.
But again, probably Eli will suggest a better solution for your use case.


On August 4, 2019 9:39:06 PM GMT+02:00, Juri Linkov <juri@linkov.net> wrote:
>> Hi, sorry, I don't understand actually why is so complex this
>provided code
>> in the email. The initialization for display-fill-column-indicator
>makes
>> some checks to set the default character as described in the
>documentation,
>> so no extra code is needed for that in the user side.
>> In the initialization I see in this mail, they just set the column's
>value
>> to 78 which can be done also using the variable fill-column for the
>whole
>> major mode too. And actually dfci will recognize it by default and
>other
>> functionalities too so in the general scenario is better to use that
>one.
>>
>> (setq fill-column 78)
>> (display-fill-column-indicator t)
>>
>> Should work no matters the order. Maybe as you were setting the
>mode's
>> variable instead of calling the function with the same name; the mode
>> was not properly initialized.
>
>The problem is that is when the need is to enable dfci, it is simpler
>to avoid eval to enable this mode because dfci works fine without using
>dfci-mode, i.e. when only variables are set in Local Variables:
>
>  ;;; Local Variables:
>  ;;; display-fill-column-indicator: t
>  ;;; display-fill-column-indicator-column: 78
>  ;;; End:
>
>or in .dir-locals.el:
>
>  (display-fill-column-indicator . t)
>  (display-fill-column-indicator-column . 78)
>
>But the display-fill-column-indicator-character can't be set
>in Local Variables because its value depends on the current display:
>either U+2502 or ?| if the font does not support Unicode characters.
>
>Do you think it would be possible to set the default value of
>display-fill-column-indicator-character without calling
>display-fill-column-indicator-mode?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-02  9:10     ` Eli Zaretskii
  2019-08-03 22:31       ` Juri Linkov
@ 2019-08-05 21:43       ` Juri Linkov
  1 sibling, 0 replies; 16+ messages in thread
From: Juri Linkov @ 2019-08-05 21:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 36861

severity 36861 wishlist
tags 36861 + patch
quit

>> diff --git a/lisp/cus-start.el b/lisp/cus-start.el
>>               (display-fill-column-indicator-character display-fill-column-indicator
>> -                                 character "27.1")
>> +                                 character "27.1" :safe characterp)
>
> This is fine with me, but I don't think we should make the change in
> .dir-locals.el.

So I only added :safe in cus-start.el.  And leaving here the
patch for the time when after Emacs 27 release it could be added
to .dir-locals.el:

diff --git a/.dir-locals.el b/.dir-locals.el
index 35dc154375..4d0151f355 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -12,7 +12,9 @@
                (electric-quote-string . nil)
 	       (mode . bug-reference-prog)))
  (log-edit-mode . ((log-edit-font-lock-gnu-style . t)
-                   (log-edit-setup-add-author . t)))
+                   (log-edit-setup-add-author . t)
+                   (display-fill-column-indicator-column . 78)
+                   (mode . display-fill-column-indicator)))
  (change-log-mode . ((add-log-time-zone-rule . t)
 		     (fill-column . 74)
 		     (mode . bug-reference)))






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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-04 20:30             ` Ergus
@ 2019-08-06 14:59               ` Eli Zaretskii
  2019-08-06 17:51                 ` Ergus
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-08-06 14:59 UTC (permalink / raw)
  To: Ergus; +Cc: 36861, juri

> Date: Sun, 04 Aug 2019 22:30:05 +0200
> CC: Eli Zaretskii <eliz@gnu.org>,36861@debbugs.gnu.org
> From: Ergus <spacibba@aol.com>
> 
> Dfci was not designed to be used in this way, and the mode initialization actually sets the character, but in the
> future it could make many other dynamic tests/checks if needed. So I don't think we should modify it to work
> the other way you suggest, because for menit is not general enough. But I will follow the Eli's suggestions in
> this aspects more than my own opinion. 
> Any way, if you want to use the mode in this way (for now) you can/need to set the
> display-fill-column-indicator-character to your desired value and it should work as it is now. But I can't promise
> that it will work the same way in the future.
> But again, probably Eli will suggest a better solution for your use case.

I don't think I understand the problem.  Why doesn't just setting the
characters and the mode variable work as expected?  AFAIU, one needs
to call the mode function only if one wants Emacs to deduce the
indicator character automatically.





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-06 14:59               ` Eli Zaretskii
@ 2019-08-06 17:51                 ` Ergus
  2019-08-06 18:26                   ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Ergus @ 2019-08-06 17:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 36861, juri

On Tue, Aug 06, 2019 at 05:59:28PM +0300, Eli Zaretskii wrote:
>> Date: Sun, 04 Aug 2019 22:30:05 +0200
>> CC: Eli Zaretskii <eliz@gnu.org>,36861@debbugs.gnu.org
>> From: Ergus <spacibba@aol.com>
>>
>> Dfci was not designed to be used in this way, and the mode initialization actually sets the character, but in the
>> future it could make many other dynamic tests/checks if needed. So I don't think we should modify it to work
>> the other way you suggest, because for menit is not general enough. But I will follow the Eli's suggestions in
>> this aspects more than my own opinion.
>> Any way, if you want to use the mode in this way (for now) you can/need to set the
>> display-fill-column-indicator-character to your desired value and it should work as it is now. But I can't promise
>> that it will work the same way in the future.
>> But again, probably Eli will suggest a better solution for your use case.
>
>I don't think I understand the problem.  Why doesn't just setting the
>characters and the mode variable work as expected?  AFAIU, one needs
>to call the mode function only if one wants Emacs to deduce the
>indicator character automatically.

Hi:

It should work as you say. But the default value for
display-fill-column-indicator-character is nil until the mode function
is executed (at least once).

I think that what they want is display-fill-column-indicator-character
to be non-nil without initialization; which we cannot set without some
checks.

Probably I'll need to reword the doc string about the default value to
specify that the value is really nil and it is initialized to the other
non-nil values in the mode function.

Does it makes sense?





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-06 17:51                 ` Ergus
@ 2019-08-06 18:26                   ` Eli Zaretskii
  2019-08-06 19:25                     ` Ergus
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-08-06 18:26 UTC (permalink / raw)
  To: Ergus; +Cc: 36861, juri

> Date: Tue, 6 Aug 2019 19:51:46 +0200
> From: Ergus <spacibba@aol.com>
> Cc: juri@linkov.net, 36861@debbugs.gnu.org
> 
> >I don't think I understand the problem.  Why doesn't just setting the
> >characters and the mode variable work as expected?  AFAIU, one needs
> >to call the mode function only if one wants Emacs to deduce the
> >indicator character automatically.
> 
> Hi:
> 
> It should work as you say. But the default value for
> display-fill-column-indicator-character is nil until the mode function
> is executed (at least once).
> 
> I think that what they want is display-fill-column-indicator-character
> to be non-nil without initialization; which we cannot set without some
> checks.

Why can't we set display-fill-column-indicator-character to ?| by
default?  Invoking the mode function will still do its job, but at
least people who want to just set the variable will have a functional
feature.  Am I missing something?





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-06 18:26                   ` Eli Zaretskii
@ 2019-08-06 19:25                     ` Ergus
  2019-08-07 14:42                       ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Ergus @ 2019-08-06 19:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 36861, juri

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

We can do that... I was just trying to avoid using ?| by default in any case. 

There is also the case when a user sets ?| by default and then call the function. If the check we add is just comparing to the default value then ?| will be always overwritten so an extra internal variable will be needed to cache where the ?| comes from. But OK, if this use case seems to be general enough I can make this changes. BTW. Have you seen my comments about the display engine in the other issue?
Best,
Ergus



On August 6, 2019 8:26:22 PM GMT+02:00, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 6 Aug 2019 19:51:46 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: juri@linkov.net, 36861@debbugs.gnu.org
>> 
>> >I don't think I understand the problem.  Why doesn't just setting
>the
>> >characters and the mode variable work as expected?  AFAIU, one needs
>> >to call the mode function only if one wants Emacs to deduce the
>> >indicator character automatically.
>> 
>> Hi:
>> 
>> It should work as you say. But the default value for
>> display-fill-column-indicator-character is nil until the mode
>function
>> is executed (at least once).
>> 
>> I think that what they want is
>display-fill-column-indicator-character
>> to be non-nil without initialization; which we cannot set without
>some
>> checks.
>
>Why can't we set display-fill-column-indicator-character to ?| by
>default?  Invoking the mode function will still do its job, but at
>least people who want to just set the variable will have a functional
>feature.  Am I missing something?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-06 19:25                     ` Ergus
@ 2019-08-07 14:42                       ` Eli Zaretskii
  2020-08-09 19:18                         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-08-07 14:42 UTC (permalink / raw)
  To: Ergus; +Cc: 36861, juri

> Date: Tue, 06 Aug 2019 21:25:00 +0200
> CC: juri@linkov.net,36861@debbugs.gnu.org
> From: Ergus <spacibba@aol.com>
> 
> We can do that... I was just trying to avoid using ?| by default in any case. 

Why should we avoid that?  It might be not the best alternative, but
at least it lets users who just set the mode variable have an
indicator.  If they want a prettier display, they will have to call
the mode function instead, as they do now.  So I think this is a net
win.  Or am I missing something?

> There is also the case when a user sets ?| by default and then call the function. If the check we add is just
> comparing to the default value then ?| will be always overwritten so an extra internal variable will be needed to
> cache where the ?| comes from.

Not sure we need another variable.  After all, if the conditions
stayed the same, the result will be the same as well, and you will
overwrite the value with an identical one.  Right?





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

* bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode
  2019-08-07 14:42                       ` Eli Zaretskii
@ 2020-08-09 19:18                         ` Lars Ingebrigtsen
  0 siblings, 0 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-09 19:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Ergus, 36861, juri

Eli Zaretskii <eliz@gnu.org> writes:

>> There is also the case when a user sets ?| by default and then call
>> the function. If the check we add is just
>> comparing to the default value then ?| will be always overwritten so
>> an extra internal variable will be needed to
>> cache where the ?| comes from.
>
> Not sure we need another variable.  After all, if the conditions
> stayed the same, the result will be the same as well, and you will
> overwrite the value with an identical one.  Right?

Reading the code, if display-fill-column-indicator-character is set,
then the mode will never do its computation to see whether we can use
the prettier U+2502 │ character.  We could special-case that, though,
and if it's ?|, then we replace it with ?│ (that is, ascii bar with
Unicode bar).

That would be kinda hacky, though, wouldn't it?  Then there would no way
to use the ASCII bar, for those that hate all things Unicode.

So we'd need another variable, unfortunately.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

end of thread, other threads:[~2020-08-09 19:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 20:44 bug#36861: 27.0.50; display-fill-column-indicator-mode in log-edit-mode Juri Linkov
2019-07-30 21:21 ` Juri Linkov
2019-07-31  2:30 ` Eli Zaretskii
2019-07-31 20:49   ` Juri Linkov
2019-08-02  9:10     ` Eli Zaretskii
2019-08-03 22:31       ` Juri Linkov
2019-08-04  0:51         ` Ergus
2019-08-04 19:39           ` Juri Linkov
2019-08-04 20:30             ` Ergus
2019-08-06 14:59               ` Eli Zaretskii
2019-08-06 17:51                 ` Ergus
2019-08-06 18:26                   ` Eli Zaretskii
2019-08-06 19:25                     ` Ergus
2019-08-07 14:42                       ` Eli Zaretskii
2020-08-09 19:18                         ` Lars Ingebrigtsen
2019-08-05 21:43       ` Juri Linkov

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