ui5-language-assistant
ui5-language-assistant copied to clipboard
Feat: implement webview to the api docs as go to definition

This pull request introduces 3 alerts when merging aaf056efb159c022e021bafe6fd739e73a616758 into 9d0c89c2d912b2c929fb3c3e7c1ac84d49ee73c1 - view on LGTM.com
new alerts:
- 3 for Unused variable, import, function or class
Hi @uxkjaer
I am a bit conflicted about this feature:
Pros:
- it is quite cool
- and may be useful.
Cons:
- It is not quite the regular "go to definition" behavior I am used to as it does not navigate to source code. It feels more like it is "go to docs" which is already implemented when you hover and then click on "more-information".
- So we have a bit of duplicate feature, (go-to-docs) which is implemented in two different ways (browser redirect vs webview inside vscode).
@petermuessig WDYT?
I think I should also consult with the product person responsible for this.
My take on it.
- youis often needs to read the api and check something.
- The hover scroll down and click isn't really useful for me.
- we currently don't have any other useful use case for the go to definition in an XML view
- further possibilities are:
- to pick up where there cursor is placed ie event or aggregation and handle deep navigation
- implement a go to samples option as well. This could be maybe implementations.
I know we aren't navigating source code, but we don't have any other useful use case for these options. We could simply add our own as well and a different short cut. My idea was that you were already used to the f12 shortcut to go and read a definition.
Good points I will setup a meeting with you and the relevant product person 👍
Hi Jacob, I think this is a good solution. There is already a similar plugin from Jacek: https://marketplace.visualstudio.com/items?itemName=jacek-wozniczak.vscode-ui5-api-reference I tried this plugin and personally I prefer to have a separate browser window. But that could also have to do with the fact that it was formatted differently here.
But you are right, I didn't know the function of "more information".
I think the use case of Jakob is a good one and can help nicely, even if I probably won't use it. But maybe it is well implemented and I will use it :)
just also wanted to bring up @wozjac's extension.
and there is also a .docset from @monavari-lebrecht at http://monavari.de/docset/ .
sooo...in best Ghostbuster's style 👻, why not combine the streams and ...
- incorporate @wozjac's extension here, into the language assistant itself (and award him eternal fame)
- provide an option to export to
.docset, taking up @monavari-lebrecht's work - have @uxkjaer's API ref in the webview as a "read more"-link of sorts, being available from the incorporated @wozjac's extension
We could also consider just using a custom menu option and command instead, if that makes more sense?
We could also consider just using a custom menu option and command instead, if that makes more sense?
Perhaps:
- Would it ever make sense to ever implement a go-to-definition that would open the source code of UI5?
- So if we keep it as currently implemented would it conflict with such a future feature?
I am generally in favor of this (or similar) feature, But it a major feature and I'd like to demonstrate it to the relevant product person, but with the holidays season in Israel now, this will add some delay...
We have types for openui5 and Sapui5 so maybe navigate to those instead?
Happy to just add this as another menu option instead. Mainly I'm just keen to get a possibility not to leave vscode when needing to search the api. We could also have another one for going to samples.
Happy to discuss this with relevant people.
I've changed the shortcut and context menu to UI5-Language-Assistant: View API Reference. The go to definition no longer shows the api reference. I've also added a setting with options to either show API reference inside vscode or in browser.
This pull request introduces 1 alert when merging 37f9155dafd2fffec972c943da0c355c04e1e8af into 90807cda6a2bb6925340057865043f97df705586 - view on LGTM.com
new alerts:
- 1 for Unused variable, import, function or class
closes #495
Does the ui5 logo belong in this PR?
I think we need to review this in a "pair programing" session. I get the feeling this PR has broken some assumptions on the code structure in this mono-repo. But I am a bit rusty in regards to this project...
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.