Re: [Nano-devel] Nano with Line Numbering

[Faissal Bensefia]

"After a quick test, it works.  But I don't like the bold -- it attracts
too much the attention.  If it were possible, I would use a greyed-out
tone."
The issue is that there needs to be something to differentiate it from
the text.

"Something that doesn't work: run 'src/nano +188,188 doc/faq.html' and
move the cursor up and down and see how things are messed up.

This line-number feature is eminently something that should be togglable:
switch on and off on the fly with an M-... key, just like syntax coloring
and whitespace display.

About the patch itself: the style doesn't match that of nano.  There
should be a space after a comma, spaces around each operator, overlong
lines should be wrapped, opening braces are on the line of the if or else,
braces are only used when needed, the indentation step is four, not two.


You use "#ifndef DISABLE_LINE_NUM", but configure.ac does not contain
anything to allow to disable the line-numbering feature."
I'll fix these later.


"In the man page you've added "set linenumbers" at the beginning, but
it should of course be in its alpabetical position.  And the new flags
(in nano.h) you've added somewhere in the middle, but those should be
at the end -- that's how it's done."
I've fixed this now.

"What is the use case for hexadecimal line numbers?"
Assembly jump alignment.

"What does below comment mean?  What do you want to represent in hex?"
Don't worry about that, I just forgot to remove that.

"And src/revision.h should not be in your patch; it should not be under
version control."
Fixed, it turns out my commits came before it was added to .gitignore

Once I've finished all the fixes, I'll post something.

~Faissal Bensefia

On 25/08/16 21:02, Benno Schulenberg wrote:
> 
> On Thu, Aug 25, 2016, at 20:22, Faissal Bensefia wrote:
>> Ok, I'll just attach the diffs for both my commits then I guess.
> 
> Thanks.
> 
> After a quick test, it works.  But I don't like the bold -- it attracts
> too much the attention.  If it were possible, I would use a greyed-out
> tone.  The vertical line is unnecessary, but kind of nice.  (Vim uses
> no line, and the color yellow by default for the line numbers.)
> 
> Something that doesn't work: run 'src/nano +188,188 doc/faq.html' and
> move the cursor up and down and see how things are messed up.
> 
> This line-number feature is eminently something that should be togglable:
> switch on and off on the fly with an M-... key, just like syntax coloring
> and whitespace display.
> 
> What is the use case for hexadecimal line numbers?
> 
> About the patch itself: the style doesn't match that of nano.  There
> should be a space after a comma, spaces around each operator, overlong
> lines should be wrapped, opening braces are on the line of the if or else,
> braces are only used when needed, the indentation step is four, not two.
> 
> What does below comment mean?  What do you want to represent in hex?
>   //Wouldn't it be cool if we added an option for hex representation?
> 
> You use "#ifndef DISABLE_LINE_NUM", but configure.ac does not contain
> anything to allow to disable the line-numbering feature.
> 
> In the man page you've added "set linenumbers" at the beginning, but
> it should of course be in its alpabetical position.  And the new flags
> (in nano.h) you've added somewhere in the middle, but those should be
> at the end -- that's how it's done.
> 
> And src/revision.h should not be in your patch; it should not be under
> version control.
> 
> Benno
>