Autodesk-AutoCAD / AutoLispExt

Visual Studio Code Extension for AutoCAD® AutoLISP
https://marketplace.visualstudio.com/items?itemName=Autodesk.autolispext
Apache License 2.0
83 stars 30 forks source link

Hover Provider #202

Closed JD-Howard closed 1 year ago

JD-Howard commented 1 year ago

Objective

I originally expected to finally address the long standing issues with AutoCompletion, but one of the prerequisites for that is providing "details" about the suggestions. So, I finished a markdown enhanced HoverProvider and the completion updates will have to be in another PR; probably for the best since this wound up having a lot of maintenance updates too.

Abstractions

The presentation layer is self contained and works for native AutoLisp/DclTiles/DclAttribute identifiers if they exist in webHelpAbstration.json or defined by the users code. This uses previous developments to get definitions from other files, but if it is used and not defined in the current file, then it does require the user to have an @Global comment to be applicable; examples of pass & fail situations provided in the video. Also note, we are a bit behind on the latest VSCode versions, the newer releases have HTML capabilities and we are restricted to Markdown right now. I don't think this is an issue, but I never got the local vscode://filePath:line# links working. So, kind of wondering if they would using newer references. For the time being, I just added a "source" to the popup to give a clue about the origin.

Uses previous developments for generating/parsing user provided documentation to enhance user defined function presentation.

There is 1 new localization, but I didn't put all the placeholders in i18n files.

Generally not married to the current presentation, but it will "do for now" until webHelpAbstraction.json gets updated. There is a test that will regenerate all the baseline MDs given a specific boolean flag. It is the obligation of whoever tinkers with the presentation code to regenerate them, manually review the outputs for "expected" results and include them in a PR to keep the tests passing. I left a similar note next to the variable.

Markdown generation does account for argument index emphasis. This will be useful for a future signature provider.

This addition makes me consider deleting the context menu "open web help" functionality in the future.

Split the original web help file into a Library & Object files. Very little changed even if it all looks like new code. Its safe to say the Library was the only thing that changed in any meaningful way.

WebStorm was used for a lot of the major refactoring.

Tests performed

The "new" content in this has ~95% code coverage, but would like to do some further testing on "User Defined" representations through mocks at some point.

All existing tests are still passing, but note that I did break a number of them during some cleanup. When your looking at test changes that aren't specifically in providers.hoverProvider.test.ts just keep in mind:

Screen shot

It is kind of hard to see my mouse in this, but I am in fact hovering over the various things that you see pop up and they often highlight the atom shortly after.

https://user-images.githubusercontent.com/51800232/188553502-db1ae887-53f2-453d-b0ff-0db86fc0d86c.mp4

github-actions[bot] commented 1 year ago

CLA Assistant Lite bot: Thank you for your submission, we really appreciate it. We ask that you sign our Contributor License Agreement before we can accept your contribution.

-->If you are contributing on behalf of your employer: you must fill out our Corporate Contributor License Agreement which can be found here, and send the signed forms to autolispext.contributor.agreements@autodesk.com.

-->If you are contributing on behalf of yourself: you must agree to our Individual Contributor License Agreement which can be found here, and either send the signed forms to autolispext.contributor.agreements@autodesk.com, or reply below with a comment containing the following text:


I have read the CLA Document and I hereby sign the CLA


Josh seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

JD-Howard commented 1 year ago

Regarding the failed tests, pretty sure this is a CRLF vs LF problem. I'll normalize the tests and try again.

I don't know what is up with your CLA email check, but I am not a fan of you trying to impose requirements that affect MY personally identifiable information. Putting my email address in public view is a hard no...

Sen-real commented 1 year ago

Regarding the failed tests, pretty sure this is a CRLF vs LF problem. I'll normalize the tests and try again.

I don't know what is up with your CLA email check, but I am not a fan of you trying to impose requirements that affect MY personally identifiable information. Putting my email address in public view is a hard no...

As the bot says, individual contributors could either send us signed CLA file or sign it online. You have already sent us the file, so please just ignore that message.

JD-Howard commented 1 year ago

That was a lot of tinkering to get those tests to pass. The CI/CD environment has a lot of things that don't work the same way as my local machine; LineEndings, readonly VSCode properties, readonly workspace settings.

Note that this has not been tested on a Mac yet, but I'll do that tomorrow. Also, not sure what is going on with CodeQL, I'll review and fix those if they ever show up. Could the CLA thing be preventing it from reporting?

Sen-real commented 1 year ago

Note that this has not been tested on a Mac yet, but I'll do that tomorrow. Also, not sure what is going on with CodeQL, I'll review and fix those if they ever show up. Could the CLA thing be preventing it from reporting?

CodeQL is temporarily turned off, and will be turned on after it works again. I'll ask CLA team if it's allowed to add an option like "I've signed CLA and don't ask me again".

JD-Howard commented 1 year ago

Mac testing is completed, 1 bug in the test itself was found.