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

feat: handling multilines results with custom output handler #5

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

brunotvs
Copy link
Contributor

@brunotvs brunotvs commented Jan 3, 2025

Fixes the multiline problem by creating a custom output handler that prepends the json output with a marker.

@HiPhish
Copy link
Owner

HiPhish commented Jan 5, 2025

I have pushed a couple of commits of my own. A couple of notes:

  • I like how you store the source to the output handler inside the output handler, that's quite clever
  • Dependencies of busted must be required lazily, that means inside the __call method. Otherwise I get an error if dkjson is not among my Lua packages when I require the output handler from within Neovim
  • I changed the result logic to search for the marked line backwards, otherwise the result logic would trip up if any of the output lines contained the result marker
  • Please do not change the code style, I intentionally put two spaces before an inline comment and no spaces inside curly braces. If you want to make stylistic changes make them in a separate commit so it can be easily reverted.

I think it should be good now. If you want you can squash the commits into one, then I can merge the PR.

Lazily `require` JSON module in output handler

We `require` the output handler in Neovim in order to get the full path
to its source.  This executes all top-level statements of the handler,
including any `require` calls.  If one of the required modules is not
installed (in this case `dkjson`) an error is raised.

The solution is to move the `require` call into the function call of the
module.  The module will not be called by Neovim, but by busted, and the
dkjson module will be present in that context.

Add commentary (motivation) to output handler

Undo stylistic changes

Shorten name of `output_handler_path` variable

Makes it easier to read.

Search for last result line containing the marker

Rename marker variable

The variable is already scoped to the output handler, so the name can be
something short.

Add tests which write to standard output

These tests are meant to be run from within Neovim.

Add test for when output contains result marker

This is why it is important to search for the output marker backwards.
@brunotvs
Copy link
Contributor Author

brunotvs commented Jan 5, 2025

Sounds cool. No experience with this squash you suggested. Hope done it right.

@brunotvs brunotvs mentioned this pull request Jan 5, 2025
@HiPhish HiPhish merged commit 9145815 into HiPhish:master Jan 5, 2025
@HiPhish
Copy link
Owner

HiPhish commented Jan 5, 2025

Yes, you have squashed everything correctly. Thank you.

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