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

 
 

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

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

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 22 Jul 2017 07:13:30 PM UTCbensOpen/ClosedOpen=>Closed
    Fri 07 Jul 2017 08:32:19 AM UTCbensStatusIn Progress=>Fixed
    Thu 06 Jul 2017 08:06:20 PM UTCbensAttached 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
    Thu 06 Jul 2017 07:21:32 PM UTCbensAttached File-=>Added 0001-rcfile-don-t-require-backslashing-in-the-argument-of.patch, #41135
    Thu 06 Jul 2017 11:09:39 AM UTCbensAttached 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.1-cleanup1