[CLOSED] Block comment command behaves unpredictably near line comments

[core-ai-bot]

Issue by peterflynn
Wednesday Dec 12, 2012 at 08:28 GMT
Originally opened as adobe/brackets#2337

---

Consider the following code:

foo();  // bar();
        // baz();

Try invoking Toggle Block Comment with the following selections:

1. Cursor in the word foo on line 1
2. Cursor in the word bar on line 1
3. Cursor in the whitespace on line 2
4. Cursor in the word baz on line 2
5. Select the word foo on line 1
6. Select the word bar on line 1
7. Select the word bar and everything after it on line 1
8. Select the middle of the whitespace on line 2
9. Select the word baz on line 2
10. Select the word baz and everything after it on line 2
11. Select from the middle of foo to the middle of bar on line 1
12. Select from the middle of foo to the end of line 1
13. Select from the middle the whitespace on line 2 to the middle of baz
14. Select from the middle the whitespace to the end of line 2

Result:

1. Empty block comment inserted (the command's usual behavior)
2. Nothing happens
3. Line uncomment
4. Line uncomment
5. Wraps selection in block comment
6. Nothing happens
7. Wraps selection in block comment
8. Line uncomment
9. Line uncomment
10. Wraps selection in block comment
11. Nothing happens
12. Wraps selection in block comment
13. Line uncomment
14. Wraps selection in block comment

Expected:
Not sure yet, but the above feels pretty muddled. In particular, it seems wrong that the pairs 6/7, 9/10, 11/12, and 13/14 differ.

[core-ai-bot]

Comment by peterflynn
Wednesday Dec 12, 2012 at 09:00 GMT

---

Here's a strawman:
If the user has selected some text, that's pretty explicit input as to what range they'd like the block comment to cover. If the range is entirely contained within a single block comment, remove it. Otherwise, wrap the entire selection in a new block comment regardless of what it contains. If there's no selection and the cursor is inside a block comment, remove it. If the cursor is on a line that consists of nothing but a line comment and whitespace, remove the line comment. In all other cases, insert an empty block comment.

I know Sublime has somewhat fancier heuristics -- for example, if there's a selection containing (parts of) multiple line comments and whitespace, but nothing else, all the line comments are removed. You could imagine being fancier still and doing something similar with multiple block comments too. But all that feels like overkill to me.

And the fancier the rules, the more unpredictable it feels: for example, in Sublime if I select a block comment plus some uncommented code on either side, it uncomments unless the selection spans multiple lines. If my selection includes two block comments, it will sometimes uncomment one while leaving the other untouched (though which one will vary), or sometimes ignores them and wrap the selected range in a new block comment... I still can't figure out what rules predict that.

[core-ai-bot]

Comment by TomMalbran
Wednesday Dec 12, 2012 at 09:32 GMT

---

The problems with the differences come from using the heuristics from line comment to uncomment. So since line comment doesn't uncomment the line comment from the first line, it creates differences in block comment.

I am still finding wrong that it does a block comment when the selection reaches the end of the line but the starts is still inside a line comment.

The idea right now is to uncomment when there is only one block comment inside a selection, complete or partial,and uncomment when the selection is inside a block comment. I like how it uncomments, because you don't need to worry about the selection to uncoment the block comment. At first it inserted block comment on invalid selection, like being part inside one block comment and part inside another, but I was told that was better not to do this, so it does nothing in those cases, some happens when the selection starts in a line comment, but it doesn't end in one or one of the middle lines wasn't a line comment. But should be pretty easy to remove this and let the user do invalid comments.

[core-ai-bot]

Comment by redmunds
Wednesday Dec 12, 2012 at 17:37 GMT

---

@peterflynn I agree that cases 6/7, 9/10 should be more consistent. I would lean toward creating a block comment in cases 6 & 9 since that's the intent of the user, but doing nothing for cases 7 & 10 would be more consistent than how it is.

I disagree with your comments on 11/12, and 13/14 because adding a block comment causes commented text to become uncommented. If you want to do that, then maybe the code should not check for validity in any case -- just blindly add a block comment around whatever is selected.

cc:@TomMalbran

[core-ai-bot]

Comment by TomMalbran
Wednesday Dec 12, 2012 at 18:17 GMT

---

/Yes. 6/7 and 9/10 sounds like bugs to me too, introduced on my last push to solve the adobe/brackets#2118. Will look into that and see what is the problem.

Okay. I got a possible fix for the 6/7 and 9/10 inconsistency. Anything else I should add/do?

[core-ai-bot]

Comment by peterflynn
Friday Dec 14, 2012 at 03:00 GMT

---

@redmunds: fyi, in Tom's pull request we eventually landed on the "maybe the code should not check for validity in any case" behavior since it seems to simplify the code. This means that you could use Toggle Block Comment to create syntactically invalid code, but that seems to be how other editors operate too.

[core-ai-bot]

Comment by TomMalbran
Friday Dec 14, 2012 at 03:55 GMT

---

@peterflynn actually it stills checks for validity for block uncommenting, it is only removed for the line uncommenting part.

[core-ai-bot]

Comment by peterflynn
Saturday Dec 15, 2012 at 00:10 GMT

---

Yep, good point Tom -- I should have remembered that we no-op on selections spanning multiple block comments (especially considering I wrote a unit test to check that case :-P)

[core-ai-bot]

Comment by peterflynn
Saturday Dec 15, 2012 at 00:12 GMT

---

Fix verified, and covered by unit tests in pull #2362. Here's the new behavior for all the cases above:

1. Inserts empty block comment at cursor pos
2. Inserts empty block comment at cursor pos
3. Line uncomment
4. Line uncomment
5. Wraps selection in block comment
6. Wraps selection in block comment
7. Wraps selection in block comment
8. Line uncomment
9. Line uncomment
10. Line uncomment
11. Wraps selection in block comment
12. Wraps selection in block comment
13. Line uncomment
14. Line uncomment

Much more consistent!