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

[Mike Scalora]

On Sun, May 1, 2016 at 1:55 PM, Benno Schulenberg
<address@hidden> wrote:
>
> 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.

ok.

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

done.

>> > +/* Name of the syntax file type used by default. */
>> > +#define DEFAULT_SYNTAX "default"
>
> This is still in the current patch.  Please remove it.

done.

>> > +    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?

\\ produces a single \

>> maybe I shouldn't try to reuse pick_up_name()
>
> Maybe not.  Maybe use what is used for parsing the arguments
> of color commands.

That one has some funky rules about embedded " but not with a
space/blank after them. I don't want to break anyone's existing syntax
files or inherit the odd rule.

I would rather leave it as is if you are OK with it.

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

I will fix that when I work on indent undo.

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

done.

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

That's where I was aiming but missed, moved it.

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

There's one == without spaces from the iTerm2 4/2/16 patch. Do you want
a separate patch to fix that or let it go?

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

Gone.

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

comment_8.patch
Description: Binary data