Issues with keybindings manager

[sergeche]

Hi guys,

I just made initial implementation of Emmet toolkit (ex-Zen Coding): https://github.com/sergeche/zen-coding The plugin can be downloaded here: http://media.chikuyonok.ru/Emmet-Brackets.zip

I have a few issues with this plugin implementation:

1. The Brackets plugin loader is too aggressive on require function. Emmet toolkit itself uses require method of core object (zen_coding.require) to get internal modules. But Brackets substitutes everything that named as require with its own bundle loader so I had to manually rename this method to something else to make core work.
2. Not all keybindings work, for example, Cmd+. (e.g. Brackets does not receive events on this shortcut but shows it correctly)
3. Is it possible to create submenus in main menu item?
4. I’ve tried to hook up on Tab key to expand abbreviations: Backets correctly handled it and expanded abbreviation but then lost focus from the editor.
5. In Editor.js, the doc says that “Editor is a 1-to-1 wrapper for a CodeMirror editor instance” but it lacks support of getOption() method so I had to use _codeMirror property directly to get access for current editor’s syntax and preferences.

[njx]

Hi--thanks for filing this!

For (3), submenus are unfortunately not implemented yet, but they're on our backlog: https://trello.com/c/OrIaeTsS -- though we might get to native menus/submenus first: https://trello.com/c/EwLGRkYe

For (5), it's okay for now to go ahead and use the _codeMirror property for things that aren't directly exposed in Editor, with the understanding that these might get wrapped in some other API in the future.

For (2) and (4), could you create separate issues for these and make this issue just be about (1)? We'd like to track these different issues separately. Thanks!

[njx]

Removing sprint 14 label

[peterflynn]

(1) is a fact of life with the version of RequireJS we're using, but it looks like it might be fixed in the current version. If you're feeling especially motivated, you could try upgrading Brackets to RequireJS 2.x to see if that's true :-)

[sergeche]

OK, created issues #1558 and #1559

If you're feeling especially motivated, you could try upgrading Brackets to RequireJS 2.x to see if that's true :-)

Maybe I will :) I’ll try to spend some time on this in a few days

[DennisKehrig]

We're now on require.js 2.1.1 (#1968)

[sergeche]

@DennisKehrig does Sprint 15 contains rjs 2.1.1?

[DennisKehrig]

@sergeche No, seems like the upgrade was done shortly after releasing sprint 15.

Sprint 16 is just around the corner, though. Until then, you can use the latest code as described here: https://github.com/adobe/brackets/wiki/How-to-Hack-on-Brackets

[sergeche]

Current master branch fails to load extensions here: https://github.com/adobe/brackets/blob/master/src/utils/ExtensionLoader.js#L59 I had to hack it a bit to make work. Also, current rjs still has some issues with require calls.

Anyway, I was able to fix some of my core modules and create custom builder that concats and obfuscates a whole plugin into a single main.js. And it works :)

The plugin is available here: https://github.com/sergeche/zen-coding/downloads Works pretty good so far.

If there’s no errors, I can announce it on http://docs.emmet.io and twitter.

[DennisKehrig]

There were some changes to the paths being used for extensions... and maybe a shell update is needed to make these work (my extensions aren't being loaded either at the moment). Unfortunately I don't have access to a more recent shell right now.

[DennisKehrig]

We introduced getActiveEditor which I recommend using over getFocusedEditor. I'll test your code later today when I have access to a more recent shell again! I look forward to it. :) Thanks for keeping us posted!

[sergeche]

OK, thanks, I’ll for your review. The only thing to be implemented in Brackets plugin is a file reader. As far as I understand, brackets.fs is an alias to Node’s FS module? I need to implement a sync binary file reader.

[DennisKehrig]

Using Sprint 16 (not yet publicly available) it works great right out of the box! Both on the Mac and on Windows. Congratulations on a job well done! :)

I just get a few duplicated shortcuts:

Cannot assign Ctrl-Shift-D to io.emmet.match_pair_inward. It is already assigned to edit.deletelines command/KeyBindingManager.js:328
Cannot assign Ctrl-D to io.emmet.match_pair_outward. It is already assigned to edit.duplicate command/KeyBindingManager.js:328
Cannot assign Alt-Up to io.emmet.increment_number_by_01. It is already assigned to navigate.previousMatch command/KeyBindingManager.js:328
Cannot assign Alt-Down to io.emmet.decrement_number_by_01. It is already assigned to navigate.nextMatch command/KeyBindingManager.js:328

And this:

Uncaught Endless loop detected extensions/user/emmet/main.js:1

Brackets isn't yet using Node.js, that's only the case for the in-browser branch. The Chromium based shell provides access to the native file system with a custom API, exposed via brackets.fs. A Node.js based version would wrap node's file system API accordingly.

I'm not sure it's possible to add synchronous functions to the shell, but it might be. Does it need to be synchronous? I'd suggest opening a new ticket or Google Groups thread for the binary reader feature, so the rest of the team can chime in. Thanks a lot!

[sergeche]

Thanks :) I’ve updated and re-uploaded Brackets plugin: no more file-based actions and changed keymap so there should be no duplicates.

As for endless loop error, it’s a protection against invalid abbreviations (or buggy parser). Can you post an abbreviation that you’re trying to expand that leads to this error?

As for sync file reader: Emmet currently uses sync calls in actions. I think I can switch to async pattern, but a bit later.

So, I think Brackets plugin is stable and can be announced :)

[peterflynn]

@sergeche: that's awesome!

If you want to do the honors of announcing it yourself, I'd suggest this:

• Add a link to Brackets extensions list. Normally people link to their repo, but most repos are usable directly without a build step... so it may be less confusing in this case if you link to the pre-built download.
• Mention your extension on the Brackets forum -- this thread, perhaps?
• Brackets Sprint 16 is about to be released. It has a few API changes, so you may want to do a sanity test once we post it (likely tomorrow).

[sergeche]

@peterflynn I would really appreciate, if Adobe Team announce it :) I can wait for String 16 to make final tests.

[peterflynn]

@sergeche: We haven't done the official announcement yet, but the build is now done and posted here: https://github.com/adobe/brackets/downloads

[sergeche]

@peterflynn just checked with Sprint 16: everything works fine. I’ve slightly updated keymap and re-uploaded plugin to https://github.com/sergeche/zen-coding/downloads

[peterflynn]

Closing this since issue (1) here is now fixed, and the others have either been spun off as bugs or recorded in the backlog. @sergeche, we have a lot of users who are excited to start using your extension now!

[sergeche]

Just a quick note: I moved repo to new location: https://github.com/emmetio/emmet Brackets plugin can be downloaded here: https://github.com/emmetio/emmet/downloads

[DennisKehrig]

Hey @sergeche,

you're giving me an incentive to support ZIP files in the extension manager. ;) Yours is the first that is not directly installable via git clone.

Since I just saw the download description - "Emmet for Adobe Brackets v1.0 (beta)": I recently read that apparently we try to not refer to Brackets as "Adobe Brackets", but just Brackets. The idea is that the project should stand on its own. Of course you can do with that piece of information whatever you want. :)

[DennisKehrig]

That was quick! Thanks!

[sergeche]

@DennisKehrig no problem, just renamed the plugin: https://github.com/emmetio/emmet/downloads

[DennisKehrig]

@sergeche Re: infinite recursion - just select an opening HTML tag, like <div> and hit tab. While this will correctly indent the tab, it'll also cause an endless loop error on the console.

[sergeche]

Oh, it breaks code indentation as well. I’m fixing it right now.

[sergeche]

Fixed this bug and re-uploaded plugin. But I had to duplicate _handleTabKey() method from Editor.js to correctly handle Tab key if there’s no valid abbreviation. Maybe you can create this method as reusable command of CodeMirror editor?

[DennisKehrig]

Hey Sergey,

you can replace the call to _handleTabKey() with this code:

editor._codeMirror.getOption("extraKeys").Tab(editor._codeMirror);

Is that good enough for you?

Thanks,

Dennis

[DennisKehrig]

@njx: Assigning to myself to keep track of it

[njx]

Thanks!

[sergeche]

@DennisKehrig I’ve pushed update that conditionally uses shared Tab handler. I’m not updating plugin since it’s useless for now, I guess :)

[DennisKehrig]

Sounds like a safe choice. :) While I'm not a legal expert, I think you should probably have a look at the Brackets license and add that to your copy of the function, though.

[DennisKehrig]

@sergeche Alright, I talked to the team. If you could just add a link to the license in the comment above the function, we'd appreciate it a lot!

Here's a suggestion:

 /**
  * The following function included from Brackets to handle Tab key
  * when abbreviation cannot be expanded (https://github.com/adobe/brackets/blob/master/LICENSE).
  * @private
  * Handle Tab key press.
  * @param {!CodeMirror} instance CodeMirror instance.
  */

Thank you, and thanks a lot for creating the plugin in the first place!