abaplint / vscode-abaplint

Visual Studio Code abaplint extension
https://marketplace.visualstudio.com/items?itemName=larshp.vscode-abaplint
MIT License
26 stars 11 forks source link

remotefs support #69

Closed marcellourbani closed 3 years ago

marcellourbani commented 4 years ago

I never used abaplint on my own extension because it's just too annoying with the default configuration and there's no way to provide a custom one on an abap server. It's a real shame not having interoperability in such a small ecosystem.

I was playing with the idea of putting config files in the user profile/workspace and make them look as part of the SAP filesystem. Would probably have worked for the client, which uses APIs like vscode.workspace.fs.*, but it's the language server which loads the config files, and uses fs.*, which will never work.

I have a few ideas on how to do this, will write them here to start the discussion:

Might also have some minor incompatibility in file formats but I don't think that will be a big deal.

Thoughts?

larshp commented 4 years ago

Configuration

it should sometime be possible to define a fallback config in the vscode extension configuration, https://github.com/abaplint/vscode-abaplint/issues/54

in https://github.com/abaplint/abaplint-abap-backend its also possible to define a configuration in code inspector, the ABAP part of remote fs could pick it up from there

File System the main trouble is the LSP which does not have a way to read the files initially, see https://github.com/abaplint/vscode-abaplint/issues/21

I would like to avoid calling fs.*, probably with the server part asking the client for the full content of the workspace via messaging. This would also be one step towards running this extension only in the browser

marcellourbani commented 4 years ago

Ideally the language server should be independent from the extension, so if someone can easily use it from vim/sublime/...

I think a sensible solution for this would be:

PS: we could also do a registerFsProvider and close #21 ;)

larshp commented 4 years ago

I would like to do something like the registerFsProvider

marcellourbani commented 4 years ago

Will have a go when I feel like. Afraid will have to convert readConfig to async, and some isReady method on Handler to wait for on initialization. Or set the public config in the constructor and make readAndSetConfig public

marcellourbani commented 4 years ago

@larshp I had a try last week, it kind of works but have to sort a race condition, clean up, test on windows...

The thing I don't like is that we can't expose APIs from the client until the server is initialised, so I had to delay the initialisation and await it to be finished before calling the handler.

Should I go on or you think is too invasive?

PS: I also thought about 2 new features: 1) provide a default abaplint.json from the vscode config when no file is found 2) allow multiple handlers, like:

documents.onDidChangeContent((change) => {
  getHandler(change.document.url).then(h=>h.validateDocument(change.document));
});

which would be useful in the plugin where abapgit and eclipse_plugin might have different abaplint.json. I would basically look for the abaplint,json in the roots of the abapgit projects. Would probably defer this one though as even more invasive, and pointless without the corresponding support from my plugin

larshp commented 4 years ago

try opening a pull request, will take a look

marcellourbani commented 4 years ago

Will do as soon as I think is good enough to be merged. Marcello

On Mon, Aug 17, 2020 at 10:17 AM Lars Hvam notifications@github.com wrote:

try opening a pull request, will take a look

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/abaplint/vscode-abaplint/issues/69#issuecomment-674764082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASW6HJ6EZL4MYRB5LIICH3SBDYRLANCNFSM4OQOJLQQ .

larshp commented 3 years ago

https://github.com/abaplint/vscode-abaplint/pull/74

this is done, closing

marcellourbani commented 3 years ago

Sorry to let this hang for so long. Last time I tried was working fine on linux but not on windows (vscode uses /, fs uses \, and abaplint ends up mixing them) Fought for a bit then dropped. Was still on my radar, but you know...