mattmcmanus / atom-ember-snippets

Ember.js ES6 snippets for Atom
https://atom.io/packages/ember-snippets
MIT License
38 stars 22 forks source link

Optimized for autocomplete+ snippets #4

Closed imanghafoori1 closed 7 years ago

imanghafoori1 commented 9 years ago

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

seifsallam commented 9 years ago

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 commented 9 years ago

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

emview... emroute... ...

seifsallam commented 9 years ago

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

imanghafoori1 commented 9 years ago

I too think that em_ is pretty nasty but as a global package, huuum we have to have something special. For my personal use I remove it without mercy. but route, view, controller is common in backbone too. Any way, if you think it is better to remove them, feel free to edit them on your own.

imanghafoori1 commented 9 years ago

I first tried with the dot syntax em.view.somthing but the auto completion did not worked with dot properly. so I chose the underscores.

imanghafoori1 commented 9 years ago

For webstorm and phpstorm I do change it to dot syntax since it works there,

imanghafoori1 commented 9 years ago

sorry I forgot to upload the local changes of route.cson file.

imanghafoori1 commented 9 years ago

I think it is safer to follow the community demand of namespace and provide an optional wrapper.cson file to translate "computed_or" to "em_computed_or" for the users to use. and put the instructions of add wrapper.cson in the README file. since we live a in crowded world, nowadays, it is likely to have these conflicts around. And we are forced to use namespaces. ;)

seifsallam commented 9 years ago

I understand your point of view, but I still think the number of people who are using Ember with other MVC is pretty small, not to change the entire naming for that. If i'm wrong and the number is higher or got higher, people will complain, and then we can add namespacing (I will personally do it). But for the majority now, Ember should be first class citizen and should be treated as such.

imanghafoori1 commented 9 years ago

Ok, I upload the em_ removed version on a new branch. in a few minutes. and Note that I hear Ember experts tell that views are used rarely and apps.

seifsallam commented 9 years ago

Please don't open another branch

seifsallam commented 9 years ago

Let's just keep things consistent here

imanghafoori1 commented 9 years ago

They are ready, I think.

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

imanghafoori1 commented 9 years ago

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.

imanghafoori1 commented 9 years ago

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.