Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement AutocompleteMetadata and add autofills #28

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

JohnnyMorganz
Copy link
Contributor

@JohnnyMorganz JohnnyMorganz commented Jun 18, 2020

This PR downloads the AutocompleteMetadata XML dump from the Roblox Client Tracker and then formats it into an output which is easier to use by the rest of the extension.

It also implements the autofill for the LuaLibrary (math, string, table, etc.) and ItemStructs (Vector2, CFrame etc.) which is provided in the dump, as well as providing links to the Developer Reference within the autofill

Closes #13
Closes #6 (See below comments)

@JohnnyMorganz
Copy link
Contributor Author

One thing which has popped up is that there are sometimes multiple different implementations of the same method (For example, CFrame.new()), and they all show up as different autocompletes in the menu.

I haven't found a way to merge them together into a list yet, but I believe for Javascript/Typescript, they just use the first available one, then merge the rest as a "(+x overloads)"
image

This could be done, but you would lose the information for the different possible methods, unless ParameterInformation is implemented (#33), where the parameters are grouped into a single list which you can scroll through
image

This hides ItemStructs like RobloxScriptConnection
or RaycastResult, as they cannot really be used in code
Copy link
Owner

@Kampfkarren Kampfkarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major issues is use of any and lack of trailing commas.

Add to CHANGELOG as well.

src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Show resolved Hide resolved
@JohnnyMorganz
Copy link
Contributor Author

New commits should fix all the issues raised, as well as added parantheses autocompletion if its a function - the autocompletion also respects if there are no parameters (#16)

I also implemented the Instance.new() snippet within ItemStruct now (not constrained to Instance.new, will read any constraints provided in the AutocompleteMetadata and show Instance name completion - but this is currently only limited to Instance.new() and BrickColor.new()*)

*BrickColor.new() has a constraint set to BrickColor:any, however I don't think all the BrickColors are available in either dump right now (maybe in ReflectionMetadata?) so nothing shows up

Copy link
Owner

@Kampfkarren Kampfkarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, sorry if this seems just like a lot of busy work.

src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Outdated Show resolved Hide resolved
src/autocompleteDump.ts Show resolved Hide resolved
src/itemStruct.ts Outdated Show resolved Hide resolved
src/itemStruct.ts Outdated Show resolved Hide resolved
@JohnnyMorganz
Copy link
Contributor Author

No worries, the changes should be fixed now

Copy link
Owner

@Kampfkarren Kampfkarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, will test this alongside the rest of your PRs soon.

@Kampfkarren
Copy link
Owner

Sorry for the very long delay.

This PR works, and it's great, but I'm not a fan of the new Instance.new method. Previously, I could just type "In", see it suggest, press tab, and then it would instantly fill where I could then follow it with the class name. Now, I have to autofill Instance, then dot, then tab new. It's much slower, can you bring the old Instance.new suggestion back in some way?

@JohnnyMorganz
Copy link
Contributor Author

I've made it change the insert text of Instance into Instance.new() with autocomplete for the instance type, if that is what you were expecting?
hlx8UuEAWw

@Kampfkarren
Copy link
Owner

Yes, but you should change the suggestion text as well in that case to make it obvious.

src/itemStruct.ts Outdated Show resolved Hide resolved
Comment on lines +30 to +32
for (const paramIndex in func.parameters) {
if (func.parameters[paramIndex] !== undefined) {
const param = func.parameters[paramIndex]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be for (const param of func.parameters)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't actually do this because I need to use the paramIndex later - unless you want me to use for ... of and use Array.findIndex() afterwards?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to do something like for (const [paramIndex, param] of Object.entries(func.parameters)).

src/itemStruct.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Roblox object autofills Make Instance.new autocomplete not prompt on dot (.)
2 participants