emacs-lsp / lsp-dart

lsp-mode :heart: dart
https://emacs-lsp.github.io/lsp-dart
GNU General Public License v3.0
202 stars 24 forks source link

allow to update an outdated dart-code #49

Open zoechi opened 4 years ago

zoechi commented 4 years ago

dap-dart-setup doesn't seem to do anything if the .extension/github/Dart-Code.Dart-Code directory already exists. A way to update would be helpful. Like checking for a new version, delete the directory and run dap-dart-setup again.

Currently I delete the directory manually ocassionally and run dap-dart-setup again.

ericdallo commented 4 years ago

Yeah, It would be great to be able to do that, @yyoncho, dap-mode has this support? Do you think it's hard to implement that?

yyoncho commented 4 years ago

It is not hard. We could attack this problem on a larger scale - e. g. move the code that downloads the dap adapters in lsp-mode, integrates it with lsp-install-server which has prefix argument, etc.

zakariaz commented 3 years ago

Hi everyone,

I looked at the doc for dap-dart-setup and it says, if you pass a prefix it will redownload the extension. @zoechi, did you trie it?

@ericdallo i searched for the function in lsp-dart-dap.el to see it implementation but i couldn't find it? i looked at every file that lsp-dart-dap.el requires and it isn't anywhere. Since you tagged it as good first issue, i wanted to try it. Any idea where can i find it?

ericdallo commented 3 years ago

@zakariaz all dap integration for lsp-dart is in lsp-dart-dap.el file, the rest is done by dap-mode, like create dynamically the dap-dart-setup function

zakariaz commented 3 years ago

@ericdallo i searched in dap-mode.el useing M-x isearch-forward and couldn't find it. i suppose if it is created dynamically, i coud find where it is generated by doing that. maybe i am doing it wrong. can you give advice since i have never contributed to a package in emacs and i ma trying to find my way.

ericdallo commented 3 years ago

Sure, here we declare the function dynamically: https://github.com/emacs-lsp/dap-mode/blob/master/dap-utils.el#L101

zakariaz commented 3 years ago

@ericdallo Actually i think it's not that hard, dap-utils-vscode-setup-function says, if the directory is absent or you pass a prefix argument to force the reinstall then download the extention. It calls dap-utils-get-vscode-extension to do so. Unless we pass the version argument to dap-utils-get-vscode-extension, the version will be latest and it will download the latest version.

In my opinion, the function to change is dap-utils-get-vscode-extension to check if the version installed is the latest, if so do nothing, if it is not, download the latest version.

What do you think?

ericdallo commented 3 years ago

LGTM any concerns @yyoncho ?

zakariaz commented 3 years ago

@ericdallo @yyoncho Or maybe a function that wrap dap-utils-get-vscode-extension to do the check

Edit: it can also be interesting to have a separate function dap-dart-update to do the check and update the extension if a new release is available

yyoncho commented 3 years ago

Sounds good to me. Ideally, I think that we emacs packages should work against specific versions of the server package (the one we know that works) and the bump of the version should be performed manually on Emacs side. Thus, when you upgrade lsp package it will download the proper server version if it is not present. Similarly, if you download melpa stable version of lsp-dart/lsp-mode which tends to be older it will load the version of the server against which we have tested.

zakariaz commented 3 years ago

@yyoncho to paraphrase what you said. By default LSP should install the tested version by the mode. If the user desire to install another version, we have to provide him the possibility of doing that in his init file. Is that what you meant?

yyoncho commented 3 years ago

Yes. The option to install particular version is mostly for lsp-dart devs