Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized for autocomplete+ snippets #4

Conversation

imanghafoori1
Copy link

https://github.com/atom-community/autocomplete-snippets

  • note that this branch heavily depends on the package called autocomplete-snippets for it's effectiveness.
  • every snippet is preceded with "em" for Ember or ds for DS. the reason for that is to Namespace the snippets so they do not collide if another package defines snippets for model,view,.... of backbone or angular.
  • no need for --help or shorthands, thanks to the autocomplete-snippets package.
    try the snippets below:
    emView
    emArrayController
    dsModel
    emComputed
    ,...

@seifsallam
Copy link
Collaborator

Awesome work, but there are still some comments regarding the changes.

I think prefixes needs more work. Some has short hand, some has underscore, some has dashes, some has both, and some had dots

For starters, lets use full names for everything. So using Ember instead of Em, and classNames instead of classN, ...etc and so on for the other cases.

For separating the words, I don't think it's a good idea to use underscores, it's not really a JavaScript convention, so i'm leaning more toward matching syntax with the function itself. That means using CamelCase to call FixureAdapter and so on for the rest of prefixes.

For case that require use of dot, lets use dot. like computed.or

For using DS, and Ember. I don't agree that we have to namespace it. Unless there is a conflict locally, we should use direct function names. For Angular and other users, I think that would be a rare case to stumble upon. And in that instance, they can disable this snippet.

For es6, it should be import-ember instead es6, because it's a snippet for import. import in this case would be custom. Also there is no need to have export default snippet.

So as a rule, lets just match the prefix with the real syntax, that matches people's expectations. And lets ditch DS or Ember.

Finally, there should be maximum of one line spacing between text lines in all files.

@imanghafoori1
Copy link
Author

On the last commits I changed all of them to _ underscores.
And prefixes are now all uniform and consistent,

em_view_...
em_route_...
...

@seifsallam
Copy link
Collaborator

Awesome, but still. As I said above. It's better to match the prefix to the real sytanx of what people would write.

Also there are cases that you've missed like emRoute.afterModel, and ds_model_hasMany-v

I provided 2 prefixes to import and export. (bot with and without the es6_)
Let them live there, I think they are not problematic or cause any conflict.
es6_
@imanghafoori1
Copy link
Author

They are ready, I think.

Just forgive me about the extra linebreakes.
I remove them in future commits.
They now cause no overload.



'Ember willClearRender Hook':
'prefix': 'view_willClearR'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fix this. view_willClearRender

I provided to set of prefixes.
just bare with me they are useful specially for new comers to the package.
'.source.coffee':

'Ember FixtureAdapter':
'prefix': 'adapter_fixture'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixture_adapter

@imanghafoori1
Copy link
Author

I tried to group all adapters under the adapter keyword,
when the user need an adapter he simply types adap... and he will be offered with 3 types and chooses one.
ok, I also provide the other prefix to.
Good idea.

activemodel_adapter
rest_adapter
fixture_adapter
prefixes added.
willClearRender fixed.
@imanghafoori1
Copy link
Author

Again, Happy to have you as a teammate.
your comments drove the project to reach a very good point.
I continue to work on this with the accepted principals.

auto-completion had problem with @ symbol.
to avoid any conflict with route.extend "@route" removed.
New file added
@jgarth jgarth mentioned this pull request Apr 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants