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

Commenting out a region with M-; is buggy #83

Open
madeleinedaly opened this issue Sep 4, 2018 · 5 comments
Open

Commenting out a region with M-; is buggy #83

madeleinedaly opened this issue Sep 4, 2018 · 5 comments
Labels

Comments

@madeleinedaly
Copy link

This might be a regression since it only started happening for me pretty recently. I still haven't found a way to reproduce it consistently, but if I try the workflow described below a few times then it it will happen usually on the 2nd or 3rd time. This happens with both JSX and standard JavaScript syntax.

The workflow I usually follow for commenting out code is to select a region (usually a line) and then type M-;. But after a recent upgrade this same workflow results in a brief delay before the comment-start syntax (// , etc.) is inserted, but at the end of the last line of the region. So nothing ends up actually getting commented out.

My workaround for this is to override M-; by binding comment-dwim to those keys instead.

Confirmed that this issue does not exist js2-jsx-mode.

@felipeochoa
Copy link
Owner

Can you post some sample code where you run into this problem? Is it on JSX code or on JS code?

cc: @wyuenho

@madeleinedaly
Copy link
Author

@felipeochoa Thanks for the response. I've seen it happen with both, but more frequently with JS code. I'll see if I can come up with a consistent example to post here later tonight.

@wyuenho
Copy link
Contributor

wyuenho commented Sep 4, 2018

I think I understand the description but I can't reproduce. The only problem I'm aware of with the commenting PR is the uncomment regex is a bit overzealous, and sometimes it'll uncomment nested levels of // or in an edge case, thinks the {/**/} in const f = () = {/* */} is a JSX comment. Your problem seems to be related to commenting instead of uncommenting, specifically you seem to be cycling the comment positions. Are you using any other package like comment-dwim-2 that overrides coment-dwim by any chance?

@madeleinedaly
Copy link
Author

@wyuenho I am not using any other packages that affect commenting like that.

I realized that during the brief delay I mentioned (after pressing M-; to comment a region), I reflexively move point or press C-g because in a lot of cases the delay lasts long enough that I start to wonder if a key got stuck or if I didn't press down enough.

So if I don't move point or press any key at all, the region does end up getting commented out, but if I insert text or type another key command then the comment syntax (// or equivalent) gets inserted at the end of the region like I described above.

For me the issue then is the delay that happens when calling rjsx-comment-dwim. Could it be from the regex matching in that defun?

@wyuenho
Copy link
Contributor

wyuenho commented Sep 5, 2018

The reason you have to wait is that rjsx mode now comments according to the parse tree, so it has to wait for js2 mode to finish parsing. If you type C-g, it'll interrupt parsing I believe. How long is the delay for you? How large is your file? What's your js2-idle-timer-delay ? Have you tried adjusting js2-dynamic-idle-timer-adjust ?

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

No branches or pull requests

3 participants