emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Advice needed on modeline customization hack...


From: Perry E. Metzger
Subject: Re: Advice needed on modeline customization hack...
Date: Mon, 17 Apr 2017 08:53:26 -0400

On Mon, 17 Apr 2017 09:00:24 +0300 Eli Zaretskii <address@hidden> wrote:
> Thanks.  Allow me a few comments below.

Thank you for taking the time!

> > Comments solicited. I'm not fond of the name
> > "column-number-mode-starts-from-zero" by the way.
>
> How about column-number-indicator-zero-based instead?

That seems superior. I'll make the change.

> > address@hidden column-number-mode-starts-from-zero
> > +  In Column Number mode, the displayed column number begins at
> > zero at +the start of a line. If you would prefer for the
> > displayed column +number to begin at one, you may set
> > address@hidden to @code{nil}.
>
> The first sentence is ambiguous with the current bidirectional
> display, because "start of line" is ambiguous.  I suggest to use the
> wording with which we describe current-column:
>
>   ... the displayed column number counts from 0 at the left margin
> of the window.

I'll do that.

> I also think that using digits 0 and 1 is better than using the
> words, but that's my personal preference, so if you feel strongly
> about using words, I won't object.

I'm used to the usual rule in English classes that one uses the
written out words for small numbers, but this is technical
documentation and I think I'll change it to the way you prefer.

> Also, please leave 2 spaces between sentences, as we use the US
> English conventions in our documentation.

I didn't realize that translated through to the final texinfo
document. I'll change it.

> >  @item %c
> > -The current column number of point.
> > +The current column number of point, starting from zero.
> > +
> > address@hidden %C
> > +The current column number of point, starting from one.
> 
> I think "zero-based" and "one-based" is better.  Or maybe include a
> more detailed description of how the column is counted.

I'll think about this. I may use your formulation.

> Please also add a NEWS entry about this new option.

Will write one. How does one do a patch for that, given that by the
time this is applied it may be in the wrong place? Or should I just
include the blurb and whomever does the commit will add it?

> > diff --git a/lisp/simple.el b/lisp/simple.el
> > index 5f70adedc4..821880b1f3 100644
> > --- a/lisp/simple.el
> > +++ b/lisp/simple.el
> > @@ -7198,6 +7198,10 @@ column-number-mode
> >  If called from Lisp, enable the mode if ARG is omitted or nil."
> >    :global t :group 'mode-line)
> >  
> > +(defvar column-number-mode-starts-from-zero t
> > +  "When set to true, Column Number mode displays columns
> > starting from zero. +Otherwise, displayed column numbers start
> > from one.")  
> 
> This should be a defcustom, not defvar.

Easily fixed.

> I think it's best to have
> it in xdisp.c, where the feature is implemented, in which case you
> should add the necessary stuff to cus-start.el to allow its
> customization by Custom.

I thought that since the rest of the lisp side was done there
(including the minor mode) that it was better to have it in the .el
file. Do you have a strong opinion?

> What about the required change to mode_line_update_needed?

I no longer need that since I now only increment the column number for
the routine that generates the modeline string and not for the
variable in the window structure. See the patch for details.

Perry
-- 
Perry E. Metzger                address@hidden



reply via email to

[Prev in Thread] Current Thread [Next in Thread]