Autodesk-AutoCAD / AutoLispExt

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

RenameProvider & a lot of supporting infrastructure #160

Closed JD-Howard closed 3 years ago

JD-Howard commented 3 years ago

Objective

Introduced 'F2' renaming functionality. Lisp not having any explicit means of contextually importing functions/variables made any solution sketchy. After a lot of considerations the "@Global" comment documentation keyword is the official "trigger" for what will be a valid cross-document renaming target. With that said, only 1 instance (of @Global) has to exist anywhere in the workspace, project or opened documents to make the keyword applicable across all of them. Do note, any instance using it will be "verified" that the tagged keyword wasn't localized; thus canceling the global flag. Finally, this is all perfectly contextual, a series of nested, but identical variable names are all scoped and acted upon according to their scope.

I will add that this infrastructure supporting F2 was also extremely foundational (new tooling) for cleaning up and/or enhancing Goto, AutoComplete and signature assistance.

Abstractions

Performance was a major obstacle in this PR. The LispContainer parser was reverted to the original non-index targeting version, then it was optimized to death until it was a full 2x faster than the formatting parser. Next I layered on additional value enhancements such as linear indexing, comment linking, non-native symbol aggregation; which caused another round of scavenging for performance. Leading to a StringBuilder class that leaned on character codes instead of character x character assembly of strings and that finally hit a sweet spot of features vs performance.

One important concept introduced in this PR is a "Flat View" of the document tree. Through extensive testing, it was proved to be an exceptionally performant process to flatten a LispContainer. With the current version of the parser pre-tagging every single LispAtom with its flattened index, linear traversal for things such as extracting "contextual" comments becomes very performant. These flat indexes were also used to store primitive pointers to concrete objects; which has all kinds of memory, garbage collection and performance benefits. These will be the cornerstone of obtaining workspace autocompletion/signature data without major impacts to the user experience.

Configuration was another big change. Our "runTests.ts" was targeting 2 directories back further than it should have, which prevented the extension from loading, which oddly created more results than targeting the right directory. I eventually figured out this was tied to my ongoing issues with command line tests/coverage. Basically, they were running, but with a pre-opened drawing causing a scope conflict with the extension activation. There are now explicit operations in the tests to load a workspace, activate the extension, open files, and most importantly force close them before exit. It now doesn't matter if you run tests/coverage from vscode or from a command line, they should all "act" exactly the same way now...

A lot of things were touched, but I do think I've gone over everything enough times it should be a slam dunk if CI/CD doesn't baulk at my configuration changes.

Tests performed

There is definitively nothing part of this PR that isn't 95%-100% on test code coverage. In fact, the configuration work performed drastically improved the overall coverage of the root 'src' file groups.

Edit: the image below shows /src/analyze/ but that was actually renamed to /src/services/ just before the PR went up.

src aggregate

src compare

Screen shot

Here is a demo of the new feature in action

https://user-images.githubusercontent.com/51800232/125433539-d5c72753-b415-486d-9a1e-4f7f035f04f3.mp4

JD-Howard commented 3 years ago

@Sen-real & @hualin-whl all checks have passed... This is a large PR, but it is all directly related to the same difficult task... I have not tested this on my Mac yet, but I will fire it up tomorrow, verify all expected tests still pass, and behaves as expected

JD-Howard commented 3 years ago

@Sen-real , I had to update all the path normalizer(s) to favor forward slashes instead of backslashes and addressed all the CodeQL annotations. All of my stuff is passing just fine on the Mac and I did manual testing too. However, the same 4 LispFormatter tests that have always been a problem on Mac are still failing, but I am not going to dig into why; at least not in this PR.