Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

Sign up for GitHub

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

Jump to bottom

Abort visual commenting if it makes no sense #193

Closed
RaafatTurki opened this issue Aug 3, 2022 · 5 comments
Closed

Abort visual commenting if it makes no sense #193

RaafatTurki opened this issue Aug 3, 2022 · 5 comments
Labels
question Question related to integration and other stuff

Comments

@RaafatTurki
Copy link

Hey there, awesome plugin BTW!
This might not be strictly Comment.nvim specific but I'd love a pointer in the right direction.

This issue is about nested filetype support so as the readme says I'm using nvim-ts-context-commentstring which integrates great with the readme pre_hook snippet

Consider the following example

1   <html>
2     <body>
3       <script>
4       let name = "raafat"
5       function print_name(name) {
6         console.log(name)
7       }
8       </script>
9     </body>
10  </html>

Visually selecting lines 1 through 5 and commenting produces an unwanted mess:

<!-- <html> -->
<!--   <body> -->
<!--     <script> -->
<!--     let name = "Raafat" -->
<!--     function print_name(name) { -->
      console.log(name)
    }
    </script>
  </body>
</html>

My aim is to calculate the commentstring at the start and end of the visual selection and only comment if they match.
I've found out that returning %s in the pre_hook results in "aborting" (is there a proper way to do this?).
Let me know what you think.
@numToStr
Copy link
Owner

numToStr commented Aug 4, 2022

Very interesting use case. I tried and the following seems to work

require('Comment').setup({
    pre_hook = function(ctx)
        local start_range = {
            srow = ctx.range.srow,
            erow = ctx.range.srow,
            scol = ctx.range.scol,
            ecol = ctx.range.scol,
        }

        local end_range = {
            srow = ctx.range.erow,
            erow = ctx.range.erow,
            scol = ctx.range.ecol,
            ecol = ctx.range.ecol,
        }

        ctx.range = start_range
        local start_cstr = require('Comment.ft').calculate(ctx)

        ctx.range = end_range
        local end_cstr = require('Comment.ft').calculate(ctx)

        return start_cstr == end_cstr and start_cstr or ''
    end,
})

FYI, you don't need https://github.com/JoosepAlviste/nvim-ts-context-commentstring unless you need jsx (which might also get integrated in future #133)

@numToStr numToStr added the question Question related to integration and other stuff label Aug 4, 2022
@RaafatTurki
Copy link
Author

RaafatTurki commented Aug 5, 2022 •
edited
Loading

Great snippet, it works nicely!

The reaon I use nvim_ts_ctx_cs is not jsx, it's embedded js in html:
Here's a minimal testing snippet

<html>
  <body>
    <script>
      let name = "raafat"
    </script>
  </body>
</html>

Which after a while of linewise commenting turns into:

<html>
  <body>
    <script>
      <!-- // <!-- // <!-- // <!-- // let name = "raafat" --> --> --> -->
    </script>
  </body>
</html>

The same happens with js/ts in svelte files, Is this a known issue or should I debug my nvim setup?

@numToStr
Copy link
Owner

numToStr commented Aug 5, 2022

You must be hitting this issue #62 (comment), which will, sadly, always remain a limitation

@RaafatTurki
Copy link
Author

I see.
How do you reckon a nvim_ts_ctx_cs based version of the snippet you've provided would look like?

@numToStr
Copy link
Owner

numToStr commented Aug 6, 2022

Hmmmm I am not sure as I don't use ts-context-commenstring. Maybe ask the plugin author for help. I guess it would be pretty similar to what I suggested earlier; calculate the commenstring twice, first for the starting range and other for the ending range then compare them.

@RaafatTurki RaafatTurki mentioned this issue Aug 7, 2022
Closed
Repository owner locked and limited conversation to collaborators Aug 8, 2022
@numToStr numToStr converted this issue into discussion #199 Aug 8, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Assignees
No one assigned
Labels
question Question related to integration and other stuff
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@RaafatTurki @numToStr