[CLOSED] Line comment bug fix

[core-ai-bot]

Issue by heysupratim
Wednesday Nov 25, 2015 at 11:00 GMT
Originally opened as adobe/brackets#11954

---

Better line comment toggling , initiates the line comment at the start of the code and maintains the position of the cursor throughout (like sublime )

Better for lint to comment indents

Affected me so hence changed it

Will fail some editorCommandHandler tests which expect the line comment at the start of the line

fix for adobe/brackets#11953 (comment)

---

heysupratim included the following code: https://github.com/adobe/brackets/pull/11954/commits

[core-ai-bot]

Comment by abose
Wednesday Nov 25, 2015 at 11:14 GMT

---

@borax12 Thanks for contributing to brackets.
There is a slight issue here

When you include a selection with uneven cursor position, it tries to comment out in the wrong place, and toggle comment breaks.
This could be fixed if you analyze the left most line position in the selection and then use that position for inserting the comments.

[core-ai-bot]

Comment by heysupratim
Wednesday Nov 25, 2015 at 11:16 GMT

---

Ah , i see, checking it out

[core-ai-bot]

Comment by heysupratim
Wednesday Nov 25, 2015 at 11:41 GMT

---

@abose i had to push extra commits due to some code formatting issues . Hope it works now

[core-ai-bot]

Comment by abose
Wednesday Nov 25, 2015 at 11:51 GMT

---

unit tests for editor command handlers fails after this change. The unit tests needs to be modified with the new commenting structure.

[core-ai-bot]

Comment by heysupratim
Wednesday Nov 25, 2015 at 18:47 GMT

---

@abose - Rewrote the test cases , The editor command handler Unit Test Cases Pass now

[core-ai-bot]

Comment by redmunds
Wednesday Nov 25, 2015 at 20:44 GMT

---

It looks like you're changing it so that inline comments are always indented. Is that correct?

This needs to be a preference because some languages (VBScript, PHP, bash?) require that inline comment delimiter is in the first column. Then you'll need tests for both cases.

[core-ai-bot]

Comment by heysupratim
Thursday Nov 26, 2015 at 04:50 GMT

---

@redmunds yeah it is always indented . I checked the behaviour in ST2 for php and bash scripts - they are also always indented .

Does brackets have per language based preferences ?

[core-ai-bot]

Comment by abose
Thursday Nov 26, 2015 at 06:04 GMT

---

🆒 Verified, all unit tests passing now.
You could have a preference like
"dontIntentComments" {"languages":['bash']} ..
With reasonable defaultpreferences set for languages.

[core-ai-bot]

Comment by redmunds
Thursday Nov 26, 2015 at 19:51 GMT

---

You could have a preference like "dontIntentComments" {"languages":['bash']}

Brackets already has a mechanism for specifying language specific preferences, so no need to invent a new one. Take a look at this example in the How to Use Brackets doc, especially the "language" section:

    "language": {
        "javascript": {
            ...
        }
    },

As far as what to call the new preference, it depends on what the default is. Since the current default is not to indent, then a name like indentInlineComments would be consistent with existing behavior since prefs that are not defined, evaluate to "falsy". If we want to change the default to be indented, then maybe dontIndentInlineComments.

Don't forget to add new preference to Preferences section of the How to Use Brackets doc and also to the defaultPreferences.json file.

[core-ai-bot]

Comment by heysupratim
Friday Nov 27, 2015 at 05:00 GMT

---

@redmunds@abose - Ohk , will take a bit of time as i need to see how preference loading and initiating works in brackets . but i will have to research which languages take line comment indentation by default

[core-ai-bot]

Comment by TomMalbran
Friday Nov 27, 2015 at 05:06 GMT

---

If some languages require that the comments start on the first column, it should be a property of the language definition and not a preference. There can be a preference if an user decides to overwrite such behaviour or wants to go back to have comments at the start of a line.

[core-ai-bot]

Comment by abose
Saturday Nov 28, 2015 at 05:34 GMT

---

Never knew that language based pref existed 😅 Thanks@redmunds@TomMalbran

[core-ai-bot]

Comment by redmunds
Monday Nov 30, 2015 at 21:14 GMT

---

@borax12 There is already PR adobe/brackets#11574 that looks like it fixes the same issue.

@MarcelGerber What's the status of your PR?

[core-ai-bot]

Comment by abose
Tuesday Dec 01, 2015 at 07:52 GMT

---

@borax12 Could you also look at #11574 and see if Your implementation has any functionality difference with it.
#11574 also doesn't seem to implement any unit tests.

[core-ai-bot]

Comment by heysupratim
Tuesday Dec 01, 2015 at 07:56 GMT

---

i will do that@arun . I have been trying to get the per language
based setting correctly . I will also look at #11574

On 12/1/15, Arun Bose notifications@github.com wrote:

@borax12 Could you also look at #11574 and see if Your implementation has
any functionality difference with it.
#11574 also doesn't seem to implement any unit tests.

---

Reply to this email directly or view it on GitHub:
adobe/brackets#11954 (comment)

[image: Supratim Chakraborty on about.me]

Supratim Chakraborty
about.me/borax12
http://about.me/borax12