Re: [Nano-devel] Comment/Uncomment feature patch

[Benno Schulenberg]

On Sat, Apr 30, 2016, at 18:00, Mike Scalora wrote:
> On Tue, Apr 26, 2016 at 5:55 AM, Benno Schulenberg <address@hidden>
> wrote:
> > It's the other way around.  If a syntax defines a formatter, ^T invokes it;
> > otherwise, if the syntax defines a linter, ^T invokes it; otherwise, ^T
> > invokes a speller (the one defined in your nanorc, or otherwise a default
> > one).  Look at ^G again when editing a Python file, for example; one that
> > uses a syntax that defines a linter.
> 
> 
> I would never interpret the existing help text that way. I would have to
> suggest:
> 
> ^T    (F12)     Invoke the formatter, if available,
>                     otherwise invoke the linter, if available,
>                     otherwise invoke spell checker, if available

The help text doesn't work that way.  Those three lines are not one
paragraph, they are three paragraphs -- they are three independent
lines.  You apparently didn't do what I toild you to do: run nano
--syntax=python and press ^G again.  The help text will be different.
(And when it's not different, your nanorc does not include the python.nanorc.)
Also scroll further down, until  "Search next occurrence backward" and
"Search next occurrence forward".  If you haven't bound them, those
lines seem a little weird too, hanging in space.

> > Oof...  Well, we have other things too for debugging purposes only,
> > so I could hardly protest.  But at the moment such undo-stack dumping
> > wouldn't be needed: undo is working flawlessly, as far as I can tell.
> 
> I was planning on adding undo support for indent/unindent soon/next.

Then add it when you need it.

> I changed this so that users wouldn't get the wrong idea after reading the
> comment description.

I get that.  But it is secondary, not necessary to make your thing work.

> I can move it to a separate patch.

Please do so.

> > +/* Name of the syntax file type used by default. */
> > +#define DEFAULT_SYNTAX "default"

This is still in the current patch.  Please remove it.

> > +    else if (*ptr=='"') {
> > +       *storage = mallocstrcpy(NULL, ++ptr);
> > +       /* allow escaped double quotes */
> > [...]
> >
> > I don't get what this part is doing.
> 
> It supports quoted strings with escaped quotes (or other chars such as
> backslash) like:
> 
>     comment ".?\""

Ah.  I see now.   But the mechanism seems to consume /all/ backslashes.
How can you then have a backslash in the comment string?

> maybe I shouldn't try to reuse pick_up_name()

Maybe not.  Maybe use what is used for parsing the arguments
of color commands.

> but I thought quotes support
> would be appreciated by users of linter and formatter commands. I find the
> lack of standard tokenization in the nanorc a bit unsettling.

Yeah, it is.  I didn't make it.

> > You're not actually undoing anything here, you're just doing the
> > opposite of what was done.  Which will work, but it is abusing the
> > undo struct.
> 
> I'm not sure I appreciate distinction between "undoing" and "doing the
> opposite" when they result in the same change in the document.

:)

For me, undoing is to put back recorded data, doing the opposite...
But indeed, comment/uncomment doesn't need to record line data,
because it's all still there.

> I thought about refactoring undo so that ADD/DEL/ENTER/JOIN are all added
> as a list of sub-undo structs so that continuous typing (including enter)
> or continuous deleting would be undoable with a single invocation like most
> other editors & word processors I have used. I was thinking this would be
> good for undo of indent and/or justify. Sometimes undo line-by-line is more
> useful though and I didn't want to start such a radical change.

Yes, something for a later change, and probably discussion first.

> > +       undo_group *new = (undo_group *)nmalloc(sizeof(undo_group));
> > +       new->next = u->grouping;
> > +       u->grouping = new;
> > +       new->top_line = lineno;
> > +       new->bottom_line = lineno;
> >
> > Why does this grouping thing have a next element?  It is never used.
> 
> It is used when uncomment modifies discontiguous lines.

??  How can it affect discontiguous lines?  ...  Ah.  You want to allow
uncommenting a selecttion of lines even when some of those lines
aren't commented?  And an undo will put back the comments just
there where they were...  ...  It appears to work.  Nice!

> I was also thinking
> ahead about unindent and handling stray/invisible spaces and empty looking
> lines. Most lines might have just a tab but some are empty (lines) or have
> an invisible space that I would keep track of in a range of lines. I wanted
> a mechanism that can restore the file to the exact characters it was before
> but also not have tons of data for long blocks of the same changes.

I see.  The current indent feature adds indents also to empty lines,
which is wrong -- it should leave blank lines alone, not doing so leads
to trailing whitespace.

> > Run 'src/nano src/winio.c', then type: M-3 PageDown M-3.
> > Then type: M-U M-U.
> 
> The cursor is now positioned at the top line that was changed by the undo.

Yes, but that is most likely not where the cursor was when
the commenting or uncommenting action was done.  It should
put the cursor back where it was.

> Of course I don't believe it is a small and niche feature or I wouldn't
> have spent time working on it. At this point in nano's life, any new
> feature is going to used by a subset of users. The relative size and
> importance of that subset can be debated but hard to estimate ahead of time
> with much confidence. I do find other programmers I know tend to use
> comment/uncomment (and indent/unindent) when it is available, it's one of
> the features that you show someone once and they usually start using it.
> The kids (young programmers my children's age) expect it.
> 
> Googling "nano comment block" and similar variations is interesting. It's
> hard to interpret frequency from the anecdotal evidence but I believe for
> every person asking there are 100s or 1000s wondering.

Thousands?  Nah...  A few score maybe.  :)

> I also added man page changes and syntax file comment additions.

I don't like the position of the comment command.  I think it should
be at the beginning, after the header and magic and linter commands,
not among the color commands.

> Let me know what other changes you would like.

Comments should start with a capital and end with a period.
There needs to be a space after commands like 'while', and
around operands like '=='.   In the man page, sentences need
to be double spaced, and better punctuated.

Oh, and the built-in table and the content sniffing I'm not going
to accept; the only way to define the comment string will be
through the syntax files.

Benno

-- 
http://www.fastmail.com - Access your email from home and the web