CSS Code Hinting as new extension for Brackets

[zoufahl]

This extension adds basic CSS Code Hinting for Brackets. (similar to HTML Code Hinting), but uses more keys to trigger the hinting.

It differentiates between primary (alphabet) and secondary (whitespace, (semi-)colon) trigger keys to distinguish whether hints should be selected initially or not.

Works in *.css files and style-blocks in html-files
Does not work in inline-css yet, due to limited tokenizing.
Hope I didn't missed a propertyname/-value, else let me know.

[redmunds]

Andre,

Thank you for this awesome contribution to Brackets. Also, thanks for changing your code as we changed the API. Some of the changes we made to the API were based on feedback we received from you, so hopefully it helped make the code better for everyone. Let us know if you have any suggestions for improvements to the API.

I started by testing out the extension, so I have some comments about how it works based on my expectations. Let us know if you disagree with any of my assumptions. Also, let us know if any of the requests are not possible, or more difficult than they should be, using the existing API.

Thanks,
Randy

[redmunds]

When the entire (unfiltered) properties list pops up, there is a horizontal scroll bar. I'm pretty sure this requires a fix to the core Brackets code (not part of the API). List should auto-stretch to the width of the content, so there is no need for an API.

UPDATE: I opened issue #2532 for this.

[redmunds]

Here's a case where I expect the properties list to automatically pop up, but it doesn't. When starting a new rule, I type:

body {

I expect the property list to popup after typing the "{".

[redmunds]

Another case where I expect property list to automatically pop up is when adding a new property to an existing rule, for example:

body {
  color: red;
}

If I place insertion pointer (IP) at end of line with "color: red;" and press Enter, I expect property list to pop up. Note that his works if I press space. It also doesn't work for Tab.

[redmunds]

Yet another case where I expect property list to automatically pop up is after selecting a value from list. For example:

body {
  text-align
}

If I place IP after "text-align" and press ":" the list of values pops up. Very nice. But after I select something from the list, ";" is automatically added to the end, but the property list does not pop up.

Interesting that if I type in the value and press ";" then I do get the value property list. So, I think you should not automatically add ";" to solve this problem. Also, in the case of some properties (especially shorthand properties like border-style), there may be multiple tokens, so that's another reason ";" should not be automatically added.

[redmunds]

When either property or value list is popped up, the first item is selected. This causes the first item to be selected if user presses Enter or Tab to add whitespace to page. We put in a parameter just handle this specific case, so let us know if it's not working correctly.

[redmunds]

After selecting a property from list, the ":" is automatically added. This is always needed, so this is perfect.

But, a space char is also added. Whitespace is optional in this case, and some people don't want it, so this shouldn't be added automatically. Note that this would be a nice preference to have, but it needs to be left off for now.

[redmunds]

Those are all of the comments I have about how extension works. After those are resolved, I'll take a closer look at the code.

Thanks for a nice set of unit tests!

[redmunds]

Calls to SpecRunnerUtils.createMockDocument() in beforeEach() should have an associated call to SpecRunnerUtils.destroyMockEditor() in afterEach().

[peterflynn]

Or rather, this should use createMockEditor() instead of the three lines of code seen here... and then afterEach() should call SpecRunnerUtils.destroyMockEditor() :-)

[peterflynn]

@redmunds, re your 4th issue (";" should not be automatically added)... it sounds like that's starting to get into multi-valued properties, which we'd previously placed out of scope in the user story since we thought it would add so much complexity. I wonder if we should steer clear of that for now, as a result.

Although certainly we should fix the bug where you get value instead of property hints after typing the semicolon -- I wonder if that could be a bug in CSSUtils?

[redmunds]

@peterflynn The "get value instead of property list" was a typo (and not a bug). I corrected my comment above.

Regarding my comment about ";" should not be added, thanks for reminding me that case is out of scope. But, I still think that it's a simpler solution in addition to solving an inevitable future case.

So, @zoufahl, if you prefer, it's OK to continue automatically inserting the ";" separator, as long as the property list is popped up afterward.

[zoufahl]

Hi there,
i just uploaded the adjusted version.

Some thoughts on the comments:

1. Properties directly after curly bracket :
   I thought it could easily added by adding '{' to the list of secondaryKeys, but unfortunately the hintlist opens and directly closes after entering {. I have put some minutes into debugging and the problem lies wihtin the CodeHintManager.handleSelect() function.
   In line 458+ the entering of { causes the hints to close, since (at least) I use the alt-key to enter {.

if ((event.keyCode !== 32 && event.ctrlKey) || event.altKey || event.metaKey) {
    // End the session if the user presses any key with a modifier (other than Ctrl+Space).
    _endSession();
}

So this is not possible at the moment :(

1. initialSelection of hints:
   As already discussed with @redmunds on IRC the current CHM does not differentiate between tab and enter.
   I changed the method from using (previously) using secondaryKeys to use primaryKeys now to determine if the hints should be preselected. Only if primaryKeys (letters + hyphen + parentheses) are entered the hints are selected, otherwise the selection is lost.
   This way (if the hintlist is already open) hitting spacebar, tab or enter has the same effect of turning of the selection. (It should behave the same way - same for ';' and ':')
2. Whitespaces after colon, automatically added semicolon
   So far i removed the whitespace after the colon (although I'm used to add a whitespace to improve readability)
   I kept the automatically added semicolon though.
3. Multiple tokens
   I haven't spent too much time trying to implement multiple property values but I think it shouldn't be that difficult to add this kind of feature. (Given there is consent about how it should behave and if semicolon should be added, etc.)

[redmunds]

To handle tab and enter keys, this needs to be:

        this.secondaryTriggerKeys = " :;{\t\n\r";

This also needs the fix <delI suggested in @iwehrman's pull request (#2530) to CodeHintManager.js, function handleKeyEvent(editor, event) in this block:

        if (event.type === "keydown") {
            if (event.keyCode === 32 && event.ctrlKey) { // User pressed Ctrl+Space
                // ...
            } else if (_inSession(editor)) {
                // ...
            }
        } else ...

Change to:

        if (event.type === "keydown") {
            if (event.keyCode === 32 && event.ctrlKey) { // User pressed Ctrl+Space
                // ...
            } else if (_inSession(editor)) {
                // ...
            } else {
                lastChar = String.fromCharCode(event.keyCode);
            }
        } else ...

[redmunds]

This is looking good. Only a few more comments.

[zoufahl]

This is a suggestion by @redmunds to extend the triggering of hints by pressing 'Tab' or 'Enter'

[redmunds]

Filed #2539 for German keyboard issue.

[zoufahl]

Note: I removed { from the secondaryTriggerKeys.
This has 2 motivations:

1. On different keyboard-layouts { are produced differently. E.g. on german layout "Alt + 8" is used to produce {, while most of you may use "Shift + [" to produce it. So what's the issue, you may ask ? (see Issue 2539)
   Using german keyboard layout entering { would previously trigger the hintlist and immediately close it, due to current CHM.handleKeyEvent mechanic to work with keyup events.
   To ensure same behaviour on every system I've removed { from the list of triggerKeys, until this issue is adressed properly.
2. Due to the small change in CHM.handleKeyEvent proposed by @redmunds the hints can now be triggered using 'Enter'.
   In other words for the following fragment

body {

CSS property-name hints would not pop up after you enter the { , but after you hit Enter to move the cursor into the next line.

[zoufahl]

I just adjusted the unittest-descriptions and fixed the remaining bugs.
Feel free to comment/review.

[redmunds]

This looks great. Thanks for sticking with this complicated feature. Merging into master.