The Wayback Machine - https://web.archive.org/web/20221207055848/https://github.com/atom/spell-check/pull/33
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

Add feature: toggle on/off #33

Merged
merged 9 commits into from Feb 4, 2016
Merged

Add feature: toggle on/off #33

merged 9 commits into from Feb 4, 2016

Conversation

roccosportal
Copy link
Contributor

@roccosportal roccosportal commented Sep 15, 2014

I added the feature 'Toggle' to the 'Packages -> Spell Check' menu. I had a look at https://github.com/fundon/atom-minimap for the implemenation. A review would be nice, since I'm new to atom and not very good in coffeescript.

@kevinsawicki
Copy link
Member

kevinsawicki commented Sep 15, 2014

Are you trying to enable/disable for a given editor or all editors? Seems the current implementation affects all editors.

I would think it would be more useful to opt-in/out a specific editor. If you want to disable to all editors it probably just makes sense to deactivate the package completely.

@kevinsawicki kevinsawicki self-assigned this Sep 15, 2014
@roccosportal
Copy link
Contributor Author

roccosportal commented Sep 15, 2014

The current implementation enable/disable the plugin for all editor. It is meant as a short hand instead of going to Preferences, searching the plugin and then disable it. Also next start the plugin ist activated again.

In my case I want to have a quick way to disable spell check for a project which has German text files.

In both ways (all editors/one editor) my case would be solved, but the latter one seems more reasonable. I only copied the code from 'atom-minimap' (disable all editors) and I don't now (yet) how to implement it.

@roccosportal
Copy link
Contributor Author

roccosportal commented Sep 17, 2014

The implementation was actually quite easy!

@acrogenesis
Copy link

acrogenesis commented Nov 30, 2014

👍 for this

@karlek
Copy link

karlek commented Nov 11, 2015

Any update on this?

@pwaller
Copy link

pwaller commented Nov 17, 2015

Ping. I just wanted to edit a tech document document and everything has been highlighted. It's very distracting and I want to turn it off.

@roccosportal
Copy link
Contributor Author

roccosportal commented Nov 17, 2015

Well I updated the code. Merging is off my hands.

@thedaniel
Copy link
Contributor

thedaniel commented Nov 30, 2015

This still needs a spec, then it can be merged.

@roccosportal
Copy link
Contributor Author

roccosportal commented Nov 30, 2015

"spec" stands for specification? I'm not sure what you mean. Any references how the specification should look like? We are talking about a toggle on/off feature, what should I specify? Thanks!

@50Wliu
Copy link
Member

50Wliu commented Nov 30, 2015

@roccosportal a "spec" is something that tests this behavior to make sure it doesn't break in the future. See the specs folder for some examples and the specs styleguide for more pointers.

@kevinsawicki kevinsawicki removed their assignment Dec 1, 2015
@roccosportal
Copy link
Contributor Author

roccosportal commented Dec 2, 2015

Alright, I will look into it. But I'm going on vacation now, anyone feel free to write a spec.

@maxcnunes
Copy link
Contributor

maxcnunes commented Dec 23, 2015

Hi,

This is a pretty useful feature. Good job @roccosportal!! I'm trying to create the tests for it.
I'm just confused how to apply my changes to this pull request. Do I have to send the pull request to roccosportal/spell-check:master?

@maxcnunes
Copy link
Contributor

maxcnunes commented Dec 23, 2015

I sent the test to @roccosportal's fork. That seems the right approach. But since he is on vocation I'm not sure he is going to be able merging it soon.

Add test to the new feature: toggle on/of atom#33
@maxcnunes
Copy link
Contributor

maxcnunes commented Jan 5, 2016

@kevinsawicki any plan merging it to master?

@jehrhardt
Copy link

jehrhardt commented Jan 19, 2016

@roccosportal: There are some conflicts that need to be resolved before merge.

Btw. great work 👍.

@roccosportal
Copy link
Contributor Author

roccosportal commented Jan 22, 2016

Did the merge and updated the toggle off spec.

@jehrhardt
Copy link

jehrhardt commented Jan 29, 2016

@kevinsawicki Is there anything to do before this can be merged?

@lee-dohm lee-dohm added the atom label Feb 2, 2016
@thedaniel
Copy link
Contributor

thedaniel commented Feb 4, 2016

This looks great. I think that @subscriptionsOfCommands should be @commandSubscriptions but in the interest of diminishing back-and-forth, I'll just change that in a separate PR.

thedaniel added a commit that referenced this pull request Feb 4, 2016
@thedaniel thedaniel merged commit 6ed00ad into atom:master Feb 4, 2016
@0mp
Copy link

0mp commented Mar 20, 2016

When will this feature be available in Atom? I cannot find it.

@thedaniel
Copy link
Contributor

thedaniel commented Mar 21, 2016

It's only just made it into the Atom 1.7 beta release, which will probably become stable in a few weeks, but there's no set release date.

@lattatude
Copy link

lattatude commented Nov 1, 2016

I'm still not seeing this available in Atom 1.11.2

@0mp
Copy link

0mp commented Nov 1, 2016

@xenomode Click Packages > Command Palette > Toggle and search for Spell Check: Toggle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment