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

refactor utils #141

Closed
wants to merge 9 commits into from
Closed

refactor utils #141

wants to merge 9 commits into from

Conversation

aarondill
Copy link
Contributor

Refactors the tabnine.utils module to add types and fix some minor issues.

The list of commits is below. Each commit should include a description of the change (or be self-evident).

  • fix: remove global in select_range
  • fix: allow nil arguments to debounce function
  • refactor: add some basic types
  • fix: allow nil values in subset()
  • refactor!: remove utils.script_path and replace with utils.module_dir
  • refactor!: current_position now returns two values
  • refactor!: ends_with and starts_with types. Remove special case for str==''
  • refactor: finish off types

this diff is large due to the removal of a duplicate select_range function that seems to be a copy/paste error.

This adds a parameter and simplifies the logic for normalizing the v_table values
@amirbilu
Copy link
Contributor

Thanks for the contribution @aarondill, this is a big change which needs to be heavily tested.. might worth to split to multiple PRS?

This more clearly describes what it returns.
script_path is replaced with script_dir which returns a value based on the currently running file

note: module_dir relies on utils.lua's location, and will break if the file is moved
use { current_position() } for the old behavior.

This is unused in the code base. should it be removed?
…tr==''

This special case seems to be unused in the codebase and is very unintuitive.
Slight code changes, no behavior changes.
table.pack can't be used here because neovim functions will error if a `n` key is present.
@aarondill
Copy link
Contributor Author

@amirbilu sorry for never responding.

I can split this up (and probably should). I can see this being split into three PRs:

  1. script_dir -> module_dir
  2. allow nil in argument list in debounce
  3. types

the first two would be fairly small, but the third would still have a large (green) diff, as non of the functions currently have types, and would now all need comments describing the types.

@aarondill
Copy link
Contributor Author

I'm closing this PR. The behavioral changes have been done in #164, #165, and #166

I'll reimplement the rest of the types after those have been merged (to avoid conflicts)

@aarondill aarondill closed this Mar 25, 2024
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.

None yet

2 participants