Refactor #14509
Refactor #14509
Conversation
|
Whao this is a lot of changes. You did run some tests against these haven't you? About For the rest I will make individual reviews and sent my feedback one by one. |
This is just a coincidence I believe. Then when people adding LSP support to other layers, they just followed the existing pattern. After we finished the migration to |
|
@smile13241324 |
|
smile became busy at work. I took a look at the commits and this is great. 👍 The If you rebase, then I can merge this. If there are any possible improvements to be made, |
|
I will rebase a while later |
- Replaced `(set (make-local-variable 'foo) bar)` with `(setq-local foo bar)` - Replaced `(when (not foo) bar)` with `(unless foo bar)`
- Replaced `(set (make-local-variable 'foo) bar)` with `(setq-local foo bar)` - Replaced `(when (not foo) bar)` with `(unless foo bar)` - Replaced unnecessary backquote construct with simple quotation
- Refactored functions - Several functions are inlined and removed from global namespace - Moved function from `packages.el` to `func.el` - Replaced `(when (not foo) bar)` with `(unless foo bar)` - Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced `(when (not foo) bar)` with `(unless foo bar)`
- Replaced `(set (make-local-variable 'foo) bar)` with `(setq-local foo bar)`
- Replaced `(when (not foo) bar)` with `(unless foo bar)`
- Moved backend determination to `config.el` - Replaced `(when (not foo) bar)` with `(unless foo bar)` - Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form - Replaced `(set (make-local-variable 'foo) bar)` with `(setq-local foo bar)` - Removed unnecessary `progn`
- Moved backend determination to `config.el` - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced `(when (not foo) bar)` with `(unless foo bar)`
- Moved backend determination to `config.el` - Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced unnecessary backquote construct with simple quotation
- Moved backend determination to `config.el` - Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form - Removed unnecessary `progn`
- Moved backend determination to `config.el` - Refactored function - Replaced unnecessary backquote construct with simple quotation
- Moved backend determination to `config.el` - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Moved backend determination to `config.el`
- Moved backend determination to `config.el` - Replaced unnecessary backquote construct with simple quotation - Moved function from `packages.el` to `func.el`
- Moved backend determination to `config.el` - Replaced `pcase` form with only one-arm with `when` or `unless` form - Replaced unnecessary backquote construct with simple quotation
- Moved backend determination to `config.el` - Replaced unnecessary backquote construct with simple quotation - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Moved backend determination to `config.el` - Refactored function - Replaced `pcase` form with only one-arm with `when` or `unless` form
- Replaced `(set (make-local-variable 'foo) bar)` with `(setq-local foo bar)`
lebensterben
force-pushed
the
backend-check
branch
from
143edc3
to
1043eae
Compare
|
I just rebased. Let me know if you want me to combine them into one commit. |
|
Thank you for contributing to Spacemacs. sdwolfz pointed out that, since each commit describes it's changes, let's keep them separate. |
|
The The bot might be checking for differences against the upstream file: "" ; right bracket around the sole remaining completion
)) (set (make-local-variable 'truncate-lines) nil))Those changes should probably be suggested upstream, then the bot will open a PR here when it detects that they have changed upstream. |
|
@lebensterben after this PR, I get an error saying that the variable I think the problem is https://github.com/syl20bnr/spacemacs/blob/develop/layers/+lang/c-c++/layers.el#L24 A quick fix was to add |
This reverts commit dae0231. c-c++-backend is void on startup: #14509 (comment)
|
I reverted the refactoring of the c-c++ layer. I'll check the rest of the layers if they work as expected. |
|
None of the other refactored layers break Spacemacs on startup. |
|
I will take a look of c-c++ layer |
|
I think I found the cause. (when (boundp c-c++-backend)After: (when (boundp 'c-c++-backend)in There is no void variable message on startup with the single quote added. |
|
I will review each of them once again. Review progress
All done. |
This reverts commit e62d55b. This fixs syl20bnr#14509 (comment)
This reverts commit e62d55b. This fixs #14509 (comment)
|
The
(pcase python-lsp-server
('mspyls 'lsp-python-ms)
('pyright 'lsp-pyright))will always return nil because of the extra quote I get the intention but as user I can't check all other layers.
Update: As mentioned on gitter. We might have to wait for more info from @thanhvg or someone else that is experiencing a |
|
Hi, it works again I don't see any problem anymore. Just a bad timing to check out dev branch. Thanks. |
For some one who enable/disable |
|
@sunlin7 You are still able to change to other backend by changing the value of the corresponding layer variable, in dotspacemacs or in IELM. I don't understand how this causes any compatibility issue. |
|
Hi @lebensterben , The reproducing steps are:
|
|
@sunlin7 So you'd prefer to compute the value for a missing layer variable on each initialization of a layer, than just compute it once with the only additional requirements of reloading dotspacemacs on inclusion of LSP layer? Why would a typical user frequently enable and disable LSP layer or any layer? |
|
Okay, I got your point. This behavior change will require:
Document update for changing configuration layer require restarting Spacemacs and update pdmp file will be helpful for some one doesn't know this behavior change: |
pdump file should be already generated everytime when you reload dotspacemacs. So there's one single step. Does this need to be documented? No. Because whenever you add or remove a layer, you're supposed to reload dotspacemacs. |
This reverts commit dae0231. c-c++-backend is void on startup: syl20bnr#14509 (comment)
This reverts commit e62d55b. This fixs syl20bnr#14509 (comment)
config.elpackages.eltofunc.el(set (make-local-variable 'foo) bar)with(setq-local foo bar)(when (not foo) bar)with(unless foo bar)pcaseform with only one-arm withwhenorunlessformprogn(cond ((eq foo) bar))form with(pcase (foo bar))(if foo bar nil)with(when foo bar)