atom / snippets

Atom snippets package
MIT License
205 stars 100 forks source link

Add a settings view where one can disable snippets for certain packages #196

Closed dralletje closed 7 years ago

dralletje commented 8 years ago

I know it does look kind of hacky, but this is the way it should be done I think. I am yet to dive deeper so we can reload without restarting it, maybe be just fully reloading all snippets. Made this because of this issue #55

PierBover commented 8 years ago

Hey @dralletje

I'm in the process of working in another PR for this repo and you will need to add tests (specs) to your fork.

I'm not part of the atom team btw... but I'm undergoing this process which is completely new to me.

aryzing commented 7 years ago

Is there anything preventing the merge?

50Wliu commented 7 years ago

Yes, it needs specs.

dralletje commented 7 years ago

Hey @50Wliu (not sure if I need to ping you), but is this enough spec that I added? 😁 I know it is only one test, I'm not very good with tests yet

krabbypattified commented 7 years ago

Eagerly awaiting this feature.. any updates?

kvsm commented 7 years ago

Just popping in to say that it's 2017 and this default snippet malarkey is still the thing which annoys me most frequently in life. Pretty please can this be reviewed (and merged). :smile:

lee-dohm commented 7 years ago

The concern I have about this is that you have to restart for things to take effect. As the PR currently stands, I am 👎 on merging because of this. The system that was used to disable keybindings doesn't have this problem. Perhaps you can model this ability on that PR? https://github.com/atom/atom/pull/8130

dralletje commented 7 years ago

Thanks for the feedback, @lee-dohm , I share the concern. Let me come back to this later this week with a restart-less version :-)

lee-dohm commented 7 years ago

Sounds good! Thanks very much for your patience and effort on this @dralletje :bow: ✨

NanerLee commented 7 years ago

so long time... Eagerly awaiting this feature

lee-dohm commented 7 years ago

@NanerLee If you would like to see this happen, perhaps you could take a stab at improving the PR?

layonferreira commented 7 years ago

Hi, I have added the ability to reload snippets without the need to restart on #242

dralletje commented 7 years ago

@layonferreira Looks awesome! Thanks for picking up where I forgot hahaha 😁 First contribution including a test, good job :)

dralletje commented 7 years ago

Closing in favor of #242