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

Fixes for undefined names #12

Merged
merged 7 commits into from
Nov 25, 2024
Merged

Fixes for undefined names #12

merged 7 commits into from
Nov 25, 2024

Conversation

PaulKlint
Copy link
Member

This pull requests defines code fixes for undefined names. The beauty is that all users of TypePal can directly profit from this. I have implemented a Levenshtein distance metric (that closely follows the formal definition, but is still efficient), to compute the distance between a given variable in a set of possible IdRoles, with what is available in the given TModel.

My questions:

  • @jurgenvinju : is this how you envisaged this stylistically?
  • I now arbitrarily cutoff suggested names with an edit distance > 3. Should we make this a TypePal configuration parameter?
  • Fixes are only generated for real errors, so the computational overhead is minimal.

Suggestions welcome (in particular about how test this in VsCode).

- For every undefined name, suggestions are generated for similar names
  in similar roles. TODO: maybe type could also be used to filter
  suggestions.
- An elegant/efficient implementation of Levensthein distance has been
  made to measure edit distance.
- When this approachs works, the other errors generated directly by
  TypePal will be reviewed for possible additions of fixes.
@DavyLandman
Copy link
Member

Great stuff! I would like to have a feature to disable this functionality if desired via the config.

int sizea = size(a);
int sizeb = size(b);

@memo
Copy link
Member

Choose a reason for hiding this comment

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

Should we configure the memo to timeout after a few minutes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes will do

@jurgenvinju
Copy link
Member

Extremely cool.

I was thinking maybe the max config parameter could be part of the require* functions that ultimately produce the code actions lists. That way a language designer can tweak it per identifier role.

@jurgenvinju
Copy link
Member

Yes this is exactly what I had in mind stylistically but I'd never dreamed this could be done generically for every typepal client so quickly. Wonder what else you have up your sleeve.

@jurgenvinju
Copy link
Member

I agree with you both that an off-switch would be good for if we apply typepal in a non-interactive setting.

@PaulKlint
Copy link
Member Author

Thanks both for reviewing. I will add TypePal configuration parameters

  • to turn this behavior off.
  • to set the cutoff (although doing this per require* would be nice, unfortunately it breaks the API. In the future when we can add keyword parameters to such functions, we can reconsider this)

@jurgenvinju
Copy link
Member

I've added an issue to fix the missing keyword parameter syntax issue. The only reason we don't have this is that we didn't want to change the syntax for a while. Internally all representations of function types already have keyword parameter names and types.

@PaulKlint PaulKlint merged commit 6a30a3b into main Nov 25, 2024
1 check passed
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.

3 participants