Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C multi-line comments #119

Closed
natemaia opened this issue Dec 31, 2019 · 16 comments
Closed

C multi-line comments #119

natemaia opened this issue Dec 31, 2019 · 16 comments

Comments

@natemaia
Copy link

Currently if you have a comment

if (test) {
    /* A comment
    * Some text
    */
    func();
}

When commenting the block using gca{ it results in a broken section

/* if (test) {*/
/*     /* A comment*/
/*     * Some text*/
/*     */*/               <---- this line
/*     func();*/
/* }*/

I realize this may be asking too much and I should just stick to single line comments or blocking them off myself. Either way thank you for all the plugins you've made over the years.

Cheers, Nate

@tpope
Copy link
Owner

tpope commented Jan 1, 2020

I recently tweaked that. Does it work if you go back one commit?

@natemaia
Copy link
Author

natemaia commented Jan 1, 2020

No, I also tried out a several other commits going back in history and it's consistently the same.

One thing I noticed

/*
* this works
* */

after commenting
/*/1* */
/** this works */
/** *1/ */

So I had a little look at the plugin and made this edit which seems to do it, I'm sure there's a cleaner way though I'm just shit with vim regex.

diff --git a/plugin/commentary.vim b/plugin/commentary.vim
index 17c285b..859119a 100644
--- a/plugin/commentary.vim
+++ b/plugin/commentary.vim
@@ -52,15 +52,15 @@ function! s:go(...) abort

   for lnum in range(lnum1,lnum2)
     let line = getline(lnum)
-    if strlen(r) > 2 && l.r !~# '\\'
+    if strlen(r) >= 2 && l.r !~# '\\'
       let line = substitute(line,
             \'\M' . substitute(l, '\ze\S\s*$', '\\zs\\d\\*\\ze', '') . '\|' . substitute(r, '\S\zs', '\\zs\\d\\*\\ze', ''),
-            \'\=substitute(submatch(0)+1-uncomment,"^0$\\|^-\\d*$","","")','g')
+            \'\=substitute(submatch(0)+1-uncomment,"^0$\\|^-\\d*\s*$","","")','g')
     endif
     if uncomment
       let line = substitute(line,'\S.*\s\@<!','\=submatch(0)[strlen(l):-strlen(r)-1]','')
     else
-      let line = substitute(line,'^\%('.matchstr(getline(lnum1),indent).'\|\s*\)\zs.*\S\@<=','\=l.submatch(0).r','')
+      let line = substitute(line,'^\%('.matchstr(getline(lnum1),indent).'\|\s*\)\zs.*\S\@<=','\=l.submatch(0)." ".r','')
     endif
     call setline(lnum,line)
   endfor

@tpope
Copy link
Owner

tpope commented Jan 1, 2020

Are you setting a custom b:commentary_format? I just noticed your */ lacks whitespace padding.

@natemaia
Copy link
Author

natemaia commented Jan 1, 2020

No, but I do have a few settings that may be related but after removing them it's still the same result, though I do get the whitespace padding, just due to

setlocal comments=sl:/*,mb:*,elx:*/ 

@natemaia
Copy link
Author

natemaia commented Jan 1, 2020

With stock commentstring and no plugin changes

before
/* some text
 * more
 */

after (still wrong)
/* /* some text*/
/*  * more*/
/*  */*/

@tpope
Copy link
Owner

tpope commented Jan 2, 2020

I can reproduce the lack of whitespace padding if one of the lines being commented out contains a */. Everything works fine if I go all the way back to 1.3. Can you confirm the same? If so, a bisect would be helpful.

@natemaia
Copy link
Author

natemaia commented Jan 2, 2020

Yea 1.3 was good here as well, here's the bisect between master and v1.3.

89f43af18692d22ed999c3097e449f12fdd8b299 is the first bad commit
commit 89f43af18692d22ed999c3097e449f12fdd8b299
Author: Chaoren Lin <chaoren@users.noreply.github.com>
Date:   Mon Oct 9 23:18:52 2017 -0400

    Fix uncommenting with irregular white spaces (#82)

    Allow stripping white spaces on the left and right independently.
    Also, make sure the stripping is not reverted by subsequent lines
    which do have white spaces.

    The following cases were broken and is now fixed:

    Before this change:

    1. ^//foo$    -> ^oo$
       ^// bar$      ^bar$
    2. ^/*foo */$ -> ^foo $
    3. ^/* foo*/$ -> ^/* /1* foo*/ */$

    After this change:

    1. ^//foo$    -> ^foo$
       ^// bar$      ^ bar$
    2. ^/*foo */$ -> ^foo$
    3. ^/* foo*/$ -> ^foo$

 plugin/commentary.vim | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

@chrisbra
Copy link

chrisbra commented Jan 3, 2020

isn't that the same as #92? I made a fix once, not sure if it still applies #93

@natemaia
Copy link
Author

natemaia commented Jan 3, 2020

I don't believe so

@carlescufi
Copy link

With stock commentstring and no plugin changes

before
/* some text
 * more
 */

after (still wrong)
/* /* some text*/
/*  * more*/
/*  */*/

I get the exact same behavior myself on the current latest release, 1.3. Any chance for a fix @tpope? I really appreciate the minimalist approach when compared to other similar plugins, but the incorrect behavior with C block comments is a real issue for us low-level programmers. @natemaia bisected it here.

@carlescufi
Copy link

carlescufi commented Oct 31, 2022

Similarly, it'd be great to be able to configure line vs block comment for C files:

int foo;
foo = bar();

when running gcap you would be able to select between:

/* int foo; */
/* foo = bar(); */

and

/* int foo;
foo = bar(); */

I can instead create a new GitHub issue with this, any preferences?

@tpope tpope closed this as completed in e87cd90 Oct 31, 2022
@tpope
Copy link
Owner

tpope commented Oct 31, 2022

Similarly, it'd be great to be able to configure line vs block comment for C files:

int foo;
foo = bar();

when running gcap you would be able to select between:

/* int foo; */
/* foo = bar(); */

and

/* int foo;
foo = bar(); */

I can instead create a new GitHub issue with this, any preferences?

This gets super complicated if you want to support things like partial uncommenting. We can't just operate on the given line range; we have to parse the entire file. I don't see this happening.

@carlescufi
Copy link

carlescufi commented Nov 1, 2022

With stock commentstring and no plugin changes

before
/* some text
 * more
 */

after (still wrong)
/* /* some text*/
/*  * more*/
/*  */*/

I get the exact same behavior myself on the current latest release, 1.3. Any chance for a fix @tpope? I really appreciate the minimalist approach when compared to other similar plugins, but the incorrect behavior with C block comments is a real issue for us low-level programmers. @natemaia bisected it here.

Thanks @tpope for the commit, but this is still not functional for me. This is what I get:

        /*
         * Make lookup to check if there's a connection object in
         * CONNECT or CONNECT_AUTO state associated with passed peer LE address.
         */

Visual select the paragraph and then gc:

        /*/1* */
        /* * Make lookup to check if there's a connection object in */
        /* * CONNECT or CONNECT_AUTO state associated with passed peer LE address. */
        /* *1/ */

Maybe I misunderstood what this commit you pushed was fixing?

@carlescufi
Copy link

This gets super complicated if you want to support things like partial uncommenting. We can't just operate on the given line range; we have to parse the entire file. I don't see this happening.

Understood, thanks for the feedback.

@tpope
Copy link
Owner

tpope commented Nov 1, 2022

With stock commentstring and no plugin changes

before
/* some text
 * more
 */

after (still wrong)
/* /* some text*/
/*  * more*/
/*  */*/

I get the exact same behavior myself on the current latest release, 1.3. Any chance for a fix @tpope? I really appreciate the minimalist approach when compared to other similar plugins, but the incorrect behavior with C block comments is a real issue for us low-level programmers. @natemaia bisected it here.

Thanks @tpope for the commit, but this is still not functional for me. This is what I get:

        /*
         * Make lookup to check if there's a connection object in
         * CONNECT or CONNECT_AUTO state associated with passed peer LE address.
         */

Visual select the paragraph and then gc:

        /*/1* */
        /* * Make lookup to check if there's a connection object in */
        /* * CONNECT or CONNECT_AUTO state associated with passed peer LE address. */
        /* *1/ */

Maybe I misunderstood what this commit you pushed was fixing?

This is the intended behavior. The old result had a syntax error at /* */*/. This one doesn't, and is still reversible.

It's not trying to remove an arbitrary multi-line comment. That's back to needing to parse the entire file.

@carlescufi
Copy link

This is the intended behavior. The old result had a syntax error at /* */*/. This one doesn't, and is still reversible.

Right, as I suspected. Thanks for confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants