[CLOSED] CSS and HTML Line Comment

[core-ai-bot]

Issue by TomMalbran
Friday Nov 16, 2012 at 07:59 GMT
Originally opened as adobe/brackets#2133

---

Adding line comment to CSS and HTML as suggested here: adobe/brackets#2119

The implementation is like Sublime's where the entire line (avoiding empty spaces at the start) gets commented if there is no selection, and just does a block-comment if there is a selection.

---

TomMalbran included the following code: https://github.com/adobe/brackets/pull/2133/commits

[core-ai-bot]

Comment by redmunds
Friday Nov 16, 2012 at 22:07 GMT

---

I expected this feature to work the same as line commenting with // in a JavaScript file, so this does not works as I expected it to in these cases:

1. If I put IP in a line in a CSS file, then the line is wrapped with /* ... / as I expect. But, since it's a "line" comment feature, then I expected the entire line to be commented, so / would start at column 0, not after the indent.
2. If I select a range of text, I expect each line that has something selected to have it's line commented in it's entirely. So, with this text (with start/stop selection indicated by | char):

 ab|c
 123
 x|yz

In a JS file results in this:

// abc
// 123
// xyz

So I would expect a CSS file to be commented like this:

/* abc*/
/* 123*/
/* xyz*/

The result I get is what I wold expect from the block commenting feature:

 ab/*c
 123
 x*/yz

I don't see the purpose of this feature if it's going to do the same as block commenting.

[core-ai-bot]

Comment by peterflynn
Friday Nov 16, 2012 at 22:15 GMT

---

I could see it both ways. The current implementation here is consistent with Sublime, so it can't be that bad :-)

@redmunds, re "what's the point?" -- to me part of the motivation for this is just discoverability and muscle memory. If you hit the shortcut you're used to using for comments in JS and it does nothing in a CSS file, it feels a little broken. But there is also a difference in behavior: if you have nothing selected, the block-comment command inserts an empty comment at the cursor position instead of commenting the whole line.

[core-ai-bot]

Comment by redmunds
Friday Nov 16, 2012 at 22:40 GMT

---

@peterflynn I think that along with muscle memory comes expectation of similar behavior. To extend the case above, I think a result of:

/*
 abc
 123
 xyz
*/

is OK. The expectation for me is more about what gets commented than how it gets commented.

The other case I forgot to mention is a range selected in the middle of a line:

 abc |123| xyz

results in:

 abc /*123*/ xyz

where I expected:

/* abc 123 xyz*/

I think this would be easy to implement with existing code, so I guess it's just a matter of deciding on expected behavior.

[core-ai-bot]

Comment by redmunds
Friday Nov 16, 2012 at 22:46 GMT

---

Done with initial review.

[core-ai-bot]

Comment by TomMalbran
Saturday Nov 17, 2012 at 00:32 GMT

---

I was thinking of doing something that would make the line-comment, comment the complete lines in the selection, since for block-commenting there is the other function. But I went with Sublime's approach in this pull.

This different behavior seems easy enough to implement, since the only hard part of restoring the original selection should always do the same thing.

[core-ai-bot]

Comment by redmunds
Wednesday Nov 28, 2012 at 04:36 GMT

---

Tom,

Sorry I didn't see your updates. Be sure to add a comment after you push changes to trigger an e-mail to get the attention of the reviewer. Anyway, we're trying to close down sprint 17, so I won't get to this until maybe next week.

Thanks, Randy

[core-ai-bot]

Comment by TomMalbran
Wednesday Nov 28, 2012 at 04:56 GMT

---

@redmunds Its ok. I did figure you were all too busy finishing the main features for this sprint, and this can wait. But i didn't knew about the mail trigger, will do it next time.

Thanks

[core-ai-bot]

Comment by redmunds
Monday Dec 03, 2012 at 20:58 GMT

---

Done with review. This works great! Just a couple minor comments.

[core-ai-bot]

Comment by TomMalbran
Monday Dec 03, 2012 at 21:33 GMT

---

Thanks. Fixed the tipo, the variables naming and the if nesting.

[core-ai-bot]

Comment by redmunds
Wednesday Dec 05, 2012 at 00:37 GMT

---

Looks good. Thanks. Merging.