Re: [PATCH] state flag related modifications - TAB key status

[Benno Schulenberg]

Op 23-12-2020 om 06:52 schreef ObeliX:
> I often work on projects, where switching between tab-indention and
> space-indention is needed. thus I frequently have to verify the current
> TAB-mode when using nano. usually I type a TAB and then go back one step
> with the cursor to see if it goes back one char or jumps back tabwidth.
> but that is quite cumbersome. [...]
>
> that's why I implemented to show the current TAB-setting as state flag.
> earlier, as you pointed out, such flag was omitted on purpose because of
> possible confusion regarding 'tabgives' (thanx for bringing this feature
> to my attention), I incorporated the 'tabgives'-setting into the
> solution. maybe it could be useful for other nano-users too.

> Subject: [PATCH] added: show currently active TAB setting in state flags
> 
> the status flag also works for buffers using 'tabgives'.
> 
> flag    tab key will insert
> -----   ------------------------------
>  'T'  : a single tab character '\t'
>  't'  : the text assiged to 'tabgives'
>  '-'  : nothing (when tabgives="")
> clear : spaces according 'tabsize'

As Peter pointed out, this is rather the wrong way around.  All other
state flags show nothing when they are in their default state (which
is how I normally use nano), so it really ought to be the same for the
tabstospaces flag.

So... a space should mean <Tab> does what it normally does: it enters
a \t.  When Alt+O is toggled... well I think it should show an "O",
because that is what the other toggles (Alt+I, Alt+L, and Alt+S) do.
When 'tabgives' is active, it could show a "t" when the first character
in the tabgives string is a tab, an "s" when it is a space, and an "x"
otherwise.  But I'm open to better suggestions.

The order of IMLRS was chosen on purpose.  Auto-indentation happens at
the left, so the "I" comes first.  Softwrap happens at the right edge
(when not using --atblanks), so "S" comes last.  Breaking long lines
happens between words, so "L" is in the middle.  Those flags are
normally on for a longer time, so they are separated by two flags
that normally are on only for a short time: Mark and Recording.

Now, where does the "O" flag fit in?  To me, the most logical place
is between I and M: most often <Tab> will be used to indent something,
or to increase the indentation after auto-indentation.  Also, when "s"
is used, we want this "s" to be rather far away from the softwrap "S".


To conclude, some small remarks about the code in the patch:

> -     if (ISSET(STATEFLAGS) && (flag == AUTOINDENT ||
> -                                             flag == BREAK_LONG_LINES || 
> flag == SOFTWRAP))
> +     if (ISSET(STATEFLAGS) &&
> +         (flag == AUTOINDENT || flag == TABS_TO_SPACES ||
> +          flag == BREAK_LONG_LINES || flag == SOFTWRAP))

Do not change the whitespace in any line if you are not changing anything
else in the line.  Also, you unnecessarily wrap the first line -- just add
the extra condition at its end, then it's much clearer what gets changed.

>       waddstr(window, ISSET(BREAK_LONG_LINES) ? "L" : " ");
>       waddstr(window, recording ? "R" : " ");
>       waddstr(window, ISSET(SOFTWRAP) ? "S" : " ");
> +     const char* cT="-";
> +     if (openfile->syntax && openfile->syntax->tab) {      // tabgives mode
> +             if (openfile->syntax->tab[0]!='\0') {
> +                     if (strlen(openfile->syntax->tab)==1 &&
> +                         openfile->syntax->tab[0]=='\t') cT="T";
> +                     else
> +                             cT="t";
> +             }                                     // else tab key is 
> disabled
> +     } else                                                // non tabgives
> +    cT = ISSET(TABS_TO_SPACES) ? " " : "T";
> +     waddstr(window, cT);

This is ugly.  The tab flag should be computed at the start of the
function, and then simply have six 'waddstr' statements in a row.
That will look much better.  Also, do not use trailing comments that
cover more than one row.  Also, nano's style is to have spaces around
all operators.

> -                     wmove(topwin, 0, COLS + 2 - statelen);
> +                     wmove(topwin, 0, COLS + 1 - statelen);

This is not right.  The placeholder for the stateflags should get an
extra "x".  But it's not enough: most other things in the minibar
should get shifted one position to the left, as they are spaced the
way they are for a reason.

Also, when the state of tabstospaces is shown among the state flags,
the toggling of Alt+O should give no textual feedback any more -- see
at the end of do_toggle().

Benno

signature.asc
Description: OpenPGP digital signature