Skip to content

feat: Checking for multiline import statements in js/ts files#893

Open
stevematney wants to merge 7 commits intovim-test:masterfrom
stevematney:multiline-js-imports
Open

feat: Checking for multiline import statements in js/ts files#893
stevematney wants to merge 7 commits intovim-test:masterfrom
stevematney:multiline-js-imports

Conversation

@stevematney
Copy link
Copy Markdown

This also adds some test helpers and minor base.vim updates for Windows path normalization.

Make sure these boxes are checked before submitting your pull request:

  • Add fixtures and spec when implementing or updating a test runner
  • Update the README accordingly
  • Update the Vim documentation in doc/test.txt (Not needed.)

@stevematney stevematney force-pushed the multiline-js-imports branch from fdbdb53 to d243dca Compare April 7, 2026 20:35
@codeinabox
Copy link
Copy Markdown
Collaborator

Thank you for this. Will read through this when I get the chance.

@codeinabox
Copy link
Copy Markdown
Collaborator

I've had a quick look through this, and I really appreciate all the improvements you've added. However, could you please break this up so it's just adds support multiline imports. I've already got a draft PR (#891) for handling Windows paths, and if your PR is focused on multiline imports it makes it easier for me review.

@stevematney
Copy link
Copy Markdown
Author

I've had a quick look through this, and I really appreciate all the improvements you've added. However, could you please break this up so it's just adds support multiline imports. I've already got a draft PR (#891) for handling Windows paths, and if your PR is focused on multiline imports it makes it easier for me review.

You got it! I just wanted to be able to run tests locally to ensure I wasn't breaking things, but now that that's done, I can remove the Windows support portions.

@stevematney stevematney force-pushed the multiline-js-imports branch 4 times, most recently from a9ad500 to 1721a89 Compare April 8, 2026 17:04
@stevematney stevematney force-pushed the multiline-js-imports branch from 1721a89 to 9eb263f Compare April 8, 2026 17:41
@stevematney
Copy link
Copy Markdown
Author

I was able to remove all the OS-normalization testing code. I left some OS-related items in the actual implementation since those changes are related to the plugin and the new code functioning properly in a Windows environment.

@codeinabox codeinabox changed the title Checking for multiline import statements in js/ts files feat: Checking for multiline import statements in js/ts files Apr 11, 2026
return empty(l:current_file) ? getcwd() : fnamemodify(l:current_file, ':h')
endfunction

function! s:find_file_upward(pattern, Callback = {path -> 1}) abort
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's the purpose and context for this? Is this for handling monorepos? Currently the plug-in assumes that the working directory is root directory of a project containing the usual files ie package.json

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, this is for monorepos. In those contexts, it's not reliable to assume the base package.json has the correct dependencies, as that's much more likely to appear in the workspace package.json.

Given the intent of the plugin to be as configuration-free as possible, this should more reliably automatically select the correct runner based on the file being tested. It also shouldn't break on repos without workspaces. In that case, it should work exactly the same as it already does.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it possible for this pull request the focus is on dealing with multi-line imports and we can split up the work for monorepos and workspaces into different pull requests? I've got some ideas about that and we could discuss them in an issue or a discussion if that works.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry for my slowness to reply! I will get to this! I just have been quite busy with other things in the meantime.

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