Fixed some completions close to end of files #1356
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a helper to compute the end-of-line position for a given offset, updates existing logic to use it, and re-enables a suite of previously skipped fourslash tests by removing t.Skip() calls and updating the failing-tests registry.
- Introduced
getLineEndOfPositionand replaced relevant calls incompletions.go. - Changed an ancestor lookup to use the correct
locationparameter. - Removed
t.Skip()from multiple tests and synchronizedfailingTests.txt.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ls/completions.go | Added getLineEndOfPosition and updated two call sites |
| internal/fourslash/tests/gen/satisfiesOperatorCompletion_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionsNonExistentImport_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionsImport_umdDefaultNoCrash2_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionsImportDefaultExportCrash1_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionListInUnclosedFunction11_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionListInUnclosedFunction10_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionListInFunctionDeclaration_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionListForUnicodeEscapeName_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionAfterNewline_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/tests/gen/completionAfterNewline2_test.go | Re-enabled test by removing t.Skip() |
| internal/fourslash/_scripts/failingTests.txt | Updated list to match newly enabled/disabled tests |
Comments suppressed due to low confidence (2)
internal/ls/completions.go:2186
- The explicit semicolon after the function call is unnecessary in Go and should be removed to match the project's style conventions.
line := getLineOfPosition(file, pos);
There was a problem hiding this comment.
sorry for the noisy diff here - I sorted this list to help with diffing when comparing this list against the list of all current failing test cases. Upon reques, I can revert that change and just remove the newly passing tests surgically
There was a problem hiding this comment.
my process was a little bit manual when it comes to this, maybe there is a better one to learn what can be unskipped? cc @gabritto
There was a problem hiding this comment.
I usually just run a command in the terminal to sort/deduplicate this as part of updating. What I really need to do is to add a script or something to update the failing tests list. I'll do that soon, but meanwhile you can do whatever is easiest.
No description provided.