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

Debug improvements #16

Merged
merged 6 commits into from
Jun 4, 2024
Merged

Conversation

fnzr
Copy link
Contributor

@fnzr fnzr commented Jun 4, 2024

This PR implements the following features related to #9:

  • Adds a configuration to customize the debugger adapter in the setup function

  • Running the DAP strategy asynchronously builds the project using nio.control.future and native lua api (based on rustacean implementation of the same feature)

* Checks for the existence of neotest_builder.zig and neotest_runner.zig in the project directory and only copies the defaults if they don't exist (allows customization of the handlers and avoids deleting user files) discarded

  • Displays an error message on build errors with the vim.notify api.

  • Selection of one output if the project emits more than one binary should work on the lua side, but I haven't tested it. I don't quite understand how to get the neotest_builder.zig to generate multiple binaries

Copy link
Owner

@lawrence-laz lawrence-laz left a comment

Choose a reason for hiding this comment

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

Checks for the existence of neotest_builder.zig and neotest_runner.zig in the project directory and only copies the defaults if they don't exist (allows customization of the handlers and avoids deleting user files)

I'm a bit unsure about this, mainly because:

  • I wanted these files to be temporary, only there for the duration of a build for neotest and opaque to the user. Have you found a use case where a user might want to customize them?
  • How would updates to these files work, if the user adds them to their source control, then we won't be able to update the files with newer versions of neotest-zig, right?

lua/neotest-zig/init.lua Show resolved Hide resolved
lua/neotest-zig/init.lua Outdated Show resolved Hide resolved
@fnzr
Copy link
Contributor Author

fnzr commented Jun 4, 2024

My worry is. if the user does have the neotest_build file in his repo, we would be overwriting then deleting it. I'm ok with skipping the check if you think that's not a big deal.

@lawrence-laz
Copy link
Owner

My worry is. if the user does have the neotest_build file in his repo, we would be overwriting then deleting it. I'm ok with skipping the check if you think that's not a big deal.

It's probably unlikely, but just to be on the safe side we could rename the file to something like __neotest_build.zig?

@fnzr
Copy link
Contributor Author

fnzr commented Jun 4, 2024

nah, I think we either make a "feature" that the user can provide his own builder and read his (at the risk of running an outdated builder), or overwrite if the file exists, guaranteeing the adapter works as intended.

@lawrence-laz
Copy link
Owner

lawrence-laz commented Jun 4, 2024

nah, I think we either make a "feature" that the user can provide his own builder and read his (at the risk of running an outdated builder), or overwrite if the file exists, guaranteeing the adapter works as intended.

I meant to rename the file we have to __neotest_build.zig to reduce collision chances to minimum. Although it's probably unlikely someone will have a neotest_build.zig file in their source.

I'd go with overwriting at least for now, because there's definitely a need for being able to update those .zig files from our side.

The idea as it currently is, is that the user is still in control of their build.zig file and neotest_build.zig file only imports and "decorates" the original user file with what the neotest-zig plugin needs.

@fnzr
Copy link
Contributor Author

fnzr commented Jun 4, 2024

should be good to go

@lawrence-laz lawrence-laz merged commit 0ca82fe into lawrence-laz:main Jun 4, 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.

2 participants