Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Configure 'disableForSelector' #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jpbarto
Copy link

@jpbarto jpbarto commented Oct 23, 2016

This modification creates a single value configuration panel which makes the disableForSelector user configurable. The default value is '.comment, .string' but means the user can more easily change it through configuration.

TODO: the package only reads the configuration value at startup and does not currently listen for a value change.

…figure the disableForSelector parameter. This modification adds a single value configuration pane that is read when the package is initialized.
@igreg
Copy link

igreg commented Jul 18, 2017

@lee-dohm would you consider merging this pull-request to the main branch?

@Aerijo
Copy link
Contributor

Aerijo commented Jul 22, 2017

Is this going to happen any time soon? LaTeX needs it desperately, as mathmode is scoped to string.

@damieng
Copy link
Contributor

damieng commented Jul 24, 2017 via email

@Aerijo
Copy link
Contributor

Aerijo commented Jul 25, 2017

I don't know how to make it work like you want, but surely the average user won't ever change it (given that it has been permanantly set from the beginning). It's only people who want one or the other that will change it, and they will probably leave it be once making that change.

I really think the benefits outweigh the drawbacks here, and this feature should be made accessible to those who need it. Definitely add a warning to the option, saying that changes won't take effect until Atom is relaunched, that it is only a "work in progress" feature, but please let this happen. It really doesn't seem like @jpbarto is going to fix it any time soon.

@lee-dohm
Copy link
Contributor

lee-dohm commented Aug 1, 2017

@Aerijo we're not going to accept a PR unfinished. If someone else wanted to take this starting point and add the finishing touches including tests and support of the configuration changing after initialization, we would definitely be interested.

@Aerijo
Copy link
Contributor

Aerijo commented Aug 2, 2017

Is there a workaround I can use in the meantime then? I've tried changing the value directly, but it seems core packages can't be edited (or found) like normal ones can. I was also unsuccessful when adding a duplicate of this package to the packages folder and changing that version.

@kasunsjc
Copy link

How can I add this work around in Windows
In following guide it shows in Linux
https://atom.io/packages/atom-cform-yaml

@Aerijo
Copy link
Contributor

Aerijo commented Jan 18, 2018

@lee-dohm This PR probably needs to be resubmitted, but you mentioned

support of the configuration changing after initialization

I've learnt a lot more about how these packages all fit together, so I was testing this myself. Unfortunately, I found that autocomplete-plus itself does not acknowledge a change of .disableForSelector. I had made the value configurable, set it to update the provider value on change, and console.log'd the value each time getSuggestions() was executed.

The value of the provider did change when the settings were changed, but the behaviour (of autocomplete-plus) did not. That is, even if the value is observed by this package, changing it will not have any effect until the next restart, or autocomplete-plus is reactivated. I feel like this is more of an issue with autocomplete-plus though, so any thoughts? Should an issue be opened on that package?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants