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

Pro fancy version for coffeeScript syntax #3

Closed

Conversation

imanghafoori1
Copy link

Happy to see your comments...

'prefix': 'arrayController'
'body':
"""
_export = Ember.ArrayController.extend(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is way complicated. Can we simplify this, to make it just the bare minimum. It should only contain body

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I just had forgotten to delete them.
thank you.

@seifsallam
Copy link
Collaborator

You've done some awesome work, and i like the new file structure (except for other, and wrappers).

About having shorthand + long form

When we type, we type words not characters (same way we read). We don’t think about single characters in isolation. We don’t type f-i-n-d, we think find, and type find as a whole. And typing partial characters break this flow, as you have to be consciously thinking about what you type.

I understand your point of view. And I’ve used it before, but it didn’t turnout well, especially when collaboration with other. Everyone had their way of naming things, and their own conventions, despite having a clear guidelines. Even you couple of month not using ember, and going back you'll have all those shorthands forgotten. (Saying this from an experience)

Plus, most of the words are not that big, with the exception of a few. I simply don’t see the point in changing the way we type find for the sake of few long words.

Another main issue is that if atom fixes or supports autocomplete for snippets tomorrow, having shorthand should be deprecated, or it will generate 2 autocomplete suggestions for each snippet. So again, I appreciate the amount of effort that you've put into this, but it will be harder for you to maintain such thing.

@imanghafoori1
Copy link
Author

ok, I agree with you.
removing the short hands is as simple as removing the wrapper.cson file
and then the rest of the package would get on pretty well... ;)
of course except the --help commands which need to be modified.

*--help snippets (such as model--help) Are meant to be used in case you or forget or suspect what to type!
they can recover your memory about what to use, quickly and effortlessly.

I tried to design the package in a way that if the user is not happy with the shorthands he or she can ignore them all together.

no big deal. ;)

with the help of this nice handy package, it seems redundant to have the shorthands.
I wish there is a way to recommend it to users along side the current one.

I remove the wrappers and shorthands on the next commit.
And we will be good to go.

the body is place on only one line.
get("model")  ->  get("key")
@imanghafoori1
Copy link
Author

After all thank you for comments. sayyedi ;) shokran

@seifsallam
Copy link
Collaborator

Awesome. About the snippets, we can add it to the README file as something recommended to use with.


'Ember computed alias map':
'prefix': 'computed.map'
'body': " Ember.computed.map('${1:array_property}', (item, index) ->
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 change quotations with multiline quotations

@seifsallam
Copy link
Collaborator

Can you run the files against a coffee script linter, to clean things up, like removing extra lines, and spaces before comas, and other small stuff like that for consistency.

Other than that, and the inline comments, your work looks pretty good! 👍

@imanghafoori1
Copy link
Author

I am working on a much better branch specially optimized to work with "autocomplete-snippets" package.
I bet you`ll find that fantastic.

@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