Skip to content

changed: lsp-haskell-completion-snippets-on default: t -> lsp-enable-… #125

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

Merged

Conversation

ncaq
Copy link
Contributor

@ncaq ncaq commented Jun 10, 2021

…snippet

If lsp-enable-snippet is disabled, snippet generation will be useless, so it is preferable to turn it off.

I also had a long time trouble with setting this option to nil on lsp-haskell without disabling the snippet.
It's good to provide a variable ledge for such users.

…snippet

If lsp-enable-snippet is disabled, snippet generation will be useless, so it is preferable to turn it off.

I also had a long time trouble with setting this option to nil on lsp-haskell without disabling the snippet.
It's good to provide a variable ledge for such users.
@michaelpj
Copy link
Collaborator

What happens if lsp-enable-snippet is disabled? Do you get an error?

An alternative would be that where we actually use this variable, we could instead pass (and lsp-enable-snippet lsp-haskell-completion-snippets-on) or something?

@ncaq
Copy link
Contributor Author

ncaq commented Jun 13, 2021

No, I do not receive any error.
However, I thought it was useless to calculate snippets when I don't use them.

Also, if anything, the aim of this PR is to prevent users from being confused when they try to disable snippets.

@michaelpj
Copy link
Collaborator

I mean users shouldn't be confused since they won't see anything untoward happening. Still, this seems safe.

@michaelpj michaelpj merged commit eb37ac4 into emacs-lsp:master Jun 21, 2021
@ncaq ncaq deleted the add-inherit-snippet-option-from-lsp-mode branch June 22, 2021 09:03
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.

2 participants