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

Pavels work + unit tests #5

Merged
merged 13 commits into from
Jun 7, 2024
Merged

Pavels work + unit tests #5

merged 13 commits into from
Jun 7, 2024

Conversation

radbasa
Copy link
Collaborator

@radbasa radbasa commented Jun 4, 2024

Description

  • First step in building a dev framework for languageserver parser hook. (Move box use parser function into its own discrete function #6)
  • Imported Pavel's work as is.
  • Copied test helper utilities from languageserver
  • Add regression unit test copied from languageserver unit test test-completion.R
  • Add test for pkg[...] function name completion.

Definition of Done

  • The change is thoroughly documented.
  • The CI passes (R CMD check, linter, unit tests, spelling).
  • Any generated files have been updated (e.g. .Rd files with roxygen2::roxygenise())

@radbasa radbasa requested a review from jakubnowicki June 4, 2024 08:46
@radbasa radbasa marked this pull request as draft June 4, 2024 09:25
@radbasa radbasa marked this pull request as ready for review June 5, 2024 04:46
@radbasa
Copy link
Collaborator Author

radbasa commented Jun 5, 2024

Couldn't get R CMD Check to see the .Rprofile in CI. Decided to just run testthat::test_dir() manually.

@radbasa radbasa requested a review from Gotfrid June 5, 2024 04:55

## What Works

| `box::use()` | Code completion | Param completion | Tooltip help | As of version | Notes |
|---------------------------|:-:|:-:|:-:|--------:|:-:|
| `pkg[...]` | | | | | |
| `pkg[...]` | ✓ | | | 0.0.0.9001 | |
Copy link
Member

Choose a reason for hiding this comment

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

Here are a few comments on what else works (code completion only)

  1. box::use(pkg) also works after this change (see attached video)
  2. box::use(pkg[attach_list]) kinda works - it will load all functions from a package, not limited to the ones in the attach_list.
  3. prefix/mod[attach_list] also works - just the bare minimum code completion tho, which is based solely on text parsing: i can box::use() a non-existent function, so not sure if we should check it (see second video).
Screen.Recording.2024-06-06.at.07.46.19.mov
Screen.Recording.2024-06-06.at.07.51.54.mov

Copy link
Collaborator Author

@radbasa radbasa Jun 7, 2024

Choose a reason for hiding this comment

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

@Gotfrid , the plans are:

  1. box::use(pkg): To auto-complete on pkg$... such as dplyr$ should show dplyr$across, or dplyr$fi should show dplyr$filter.
  2. box::use(pkg[attach_list]): to only auto-complete the attached functions

Same goes for path/modules.

Then there are the aliases.

@radbasa radbasa merged commit 5837e08 into main Jun 7, 2024
5 checks passed
@radbasa radbasa deleted the pavels_work branch June 7, 2024 06:47
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.

2 participants