bugGNU nano - Bugs: bug #51370, the quoting regime in the syntax...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #51370: the quoting regime in the syntax files is inconsistent

Submitter:  Benno Schulenberg <bens>
Submitted:  Mon 03 Jul 2017 09:20:48 AM UTC
   
 
Severity:  3 - Normal Status:  Fixed
Assigned to:  bens Open/Closed:  Closed

Jump to the original submission

Fri 07 Jul 2017 08:32:19 AM UTC, comment #9: 

Fixed in git, by a7901dd1e, and completed by 79971a30.

Benno Schulenberg <bens>
Group administrator
Thu 06 Jul 2017 09:06:41 PM UTC, comment #8: 

I think I did miss it.  Tried again with the entire sequence: your new first patch, plus your other two (0001 through 0003), and they still seem to work well as a whole (after I put back my nanorcs for testing).

One minor documentation glitch, though: in patch 0002, when it rewords the description of the "comment" option in nano.texi, there's an unnecessary space: "If the string contains a vertical bar or pipe character (@t{|}), this  designates bracket-style comments;"  there are two spaces between "this" and "designates" where there should only be one.

David Lawrence Ramsey <dolorous>
Group Member
Thu 06 Jul 2017 08:06:20 PM UTC, comment #7: 

Here is the documentation patch, and a follow-on patch to simplify the comment inhibition.  (And maybe you missed my reworked first patch, as we posted almost at the same time.)

(file #41138, file #41139)

Benno Schulenberg <bens>
Group administrator
Thu 06 Jul 2017 07:23:51 PM UTC, comment #6: 

Your patch does seem to work so far; thanks.  However, while testing it, I found another bug which I had to hack around first in order to verify that your patch worked.  But I'll report that separately too.

David Lawrence Ramsey <dolorous>
Group Member
Thu 06 Jul 2017 07:21:32 PM UTC, comment #5: 

The first patch didn't do the right thing when the argument /didn't/ start with a double quote.  Attached is a second attempt.

(file #41135)

Benno Schulenberg <bens>
Group administrator
Thu 06 Jul 2017 11:09:40 AM UTC, comment #4: 

Here's a first, rough patch.  It should fix this bug (never requiring double quotes to be escaped) and bug #51394.

(It will require updating the documentation plus a few syntaxes, but that will come later.)

(file #41133)

Benno Schulenberg <bens>
Group administrator
Wed 05 Jul 2017 08:30:01 PM UTC, comment #3: 

I figured that if there was an inconsistency in the documentation regarding comment quoting, I should verify what the comment quoting code actually did, just in case something was off.  I'll go ahead and report it as a separate bug, though.

As for removing the need to backslash quotes, sounds good to me; I wasn't sure which way was preferred.

As for languages using pipes as comments, I can't think of one, and quick googling turns up nothing.  So I suppose I'm just paranoid, and an effort to escape pipes in comments should wait until there's an actual case that needs it.

David Lawrence Ramsey <dolorous>
Group Member
Wed 05 Jul 2017 07:05:14 PM UTC, comment #2: 

So... you found a bug: when the comment string is actually empty, it should have disabled commenting, but in fact it doesn't.  I think that should have been reported as a separate bug.

(But it's rather absurd that trailing junk after the first two double quotes prevents the parser from noting that the string is empty -- also «comment ""  joohoo» will fail to disable commenting.  :| )

But probably we can fix both bugs at the same type: fix the quoting regime so that double quotes don't ever need to be backslashed, and in the bargain achieve that an empty string will always be noted as such.

So... pick_up_name() should be fixed to not require an internal double quote to be quoted.  (And later I will rename it to pick_up_argument or something, because since comment() it no longer just picks up a name of a program.)

(Regarding the impossibility of using a pipe as a comment character...  Do you know any language or file type where the pipe character is used for comments?  If not, why bother generalizing the routine?)

Benno Schulenberg <bens>
Group administrator
Tue 04 Jul 2017 09:26:27 PM UTC, comment #1: 

I've tested this with current git (7473105), and parsing of the comment string is odd.  Despite what the documentation says, backslashing of double quotes is actually required for the comment option, since the comment string is parsed via rcfile.c:pick_up_name(), which requires such backslashing.  However, not backslashing double quotes leads to erroneous commenting behavior without generating errors.  You can reproduce it via the following steps:

1. Create a new syntax file in /usr/share/nano/unquoted.nanorc, containing:

syntax "unquoted"
comment """

2. Build nano from git with --sysconfdir=/etc and --prefix=/usr.

3. Run:

./nano -Y unquoted

nano won't generate any errors when it parses the "unquoted" syntax, although the syntax highlighting of the unquoted.nanorc file will color the third quote of """ bright red.

4. Type several lines' worth of text, turn the mark on and highlight those lines, and then press Meta-3.  The file will be marked as modified, but the text won't change.  (In this case, the comment string is apparently empty, although erroneously not flagged as such.)

Now, change the comment line in /usr/share/nano/unquoted.nanorc to:

comment  "\""

and repeat the above steps.  In step 4, the lines will now be commented with a ", as expected.

(For the record, the method used to separate the beginning and ending portions of a comment with a '|' is also odd.  The '|' is checked for in text.c:comment_line() with a simple strchr(), so it's impossible to use '|' as a commenting character at all.  Maybe '|' should be allowed if it's backslashed first, for maximum flexibility?  But that would make parsing it more complex and possibly break compatibility.  Not to mention, that's outside the scope of this bug.)

David Lawrence Ramsey <dolorous>
Group Member
Mon 03 Jul 2017 09:20:48 AM UTC, original submission:  

In the beginning of the nanorc manpage, it says: "Quotes inside these string parameters don't have to be escaped with backslashes."  But later on, for the "comment string" directive, it says that double quotes need to be backslashed.  That is not consistent.

Benno Schulenberg <bens>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dolorous (Posted a comment)
  • -email is unavailable- added by bens (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-22 bens Open/ClosedOpen Closed
    2017-07-07 bens StatusIn Progress Fixed
    2017-07-06 bens Attached File- Added 0002-docs-remove-the-mention-of-backslashes-for-the-argum.patch, #41138
        Attached File- Added 0003-tweaks-recognize-the-empty-string-as-comment-inhibit.patch, #41139
    2017-07-06 bens Attached File- Added 0001-rcfile-don-t-require-backslashing-in-the-argument-of.patch, #41135
    2017-07-06 bens Attached File- Added 0001-rcfile-don-t-require-backslashing-in-the-argument-of.patch, #41133
        StatusNone In Progress
        Assigned toNone bens

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code