Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

RFC - Dynamic source conditions #203

Merged

Conversation

levouh
Copy link
Contributor

@levouh levouh commented Sep 19, 2021

Adds a dynamic_condition that is evaluated at "runtime" for all sources that discerns whether or not it will be run each time they have the potential to be run.

See associated discussion here as to why this might be useful.

Some things still to do, but curious on your thoughts @jose-elias-alvarez before messing with getting these going:

  • Add tests

@levouh levouh changed the title [RFC] Dynamic source conditions WIP: +RFC - Dynamic source conditions Sep 19, 2021
@levouh levouh marked this pull request as draft September 19, 2021 23:16
@levouh levouh changed the title WIP: +RFC - Dynamic source conditions RFC - Dynamic source conditions Sep 19, 2021
lua/null-ls/generators.lua Outdated Show resolved Hide resolved
@levouh levouh force-pushed the dynamic-condition branch 2 times, most recently from 699b79a to 5cdfa05 Compare September 20, 2021 14:32
@jose-elias-alvarez
Copy link
Owner

This is a great idea and works well. Strictly speaking, since null-ls allows running generators on demand, something like this is already possible, but I think this is a clean addition and far more user-friendly.

I wonder if it might be worth it to move the dynamic_condition check to the generator loop in generators.run. This would give the callback access to params, which I imagine might be useful to simplify checking whether a source should run for the given file or not. Maybe something like this:

diff --git a/lua/null-ls/generators.lua b/lua/null-ls/generators.lua
index 41510af..9aea507 100644
--- a/lua/null-ls/generators.lua
+++ b/lua/null-ls/generators.lua
@@ -18,6 +18,12 @@ M.run = function(generators, params, postprocess, callback)
         for _, generator in ipairs(generators) do
             table.insert(futures, function()
                 local copied_params = vim.deepcopy(params)
+
+                local dynamic_condition = generator.opts.dynamic_condition
+                if dynamic_condition and not dynamic_condition(copied_params) then
+                    return
+                end
+
                 local to_run = generator.async and a.wrap(generator.fn, 2) or generator.fn
                 local ok, results = pcall(to_run, copied_params)
                 a.util.scheduler()
@@ -92,19 +98,9 @@ M.run_registered_sequentially = function(opts)
     M.run_sequentially(generators, make_params, postprocess, callback, after_all)
 end

What do you think? Other than that, I'm happy to merge this once we have tests (unit tests and a simple E2E test would be ideal).

@levouh
Copy link
Contributor Author

levouh commented Sep 22, 2021

Makes sense to me, having access to params would be nice as well. I also think a name like runtime_condition might be better as that's what we're doing, but curious on your thoughts there as well.

I'll look at this patch and adding tests to get it through sometime this week. Thanks for the feedback.

@levouh levouh force-pushed the dynamic-condition branch 2 times, most recently from 949b635 to fb0b84c Compare September 25, 2021 16:45
@levouh levouh marked this pull request as ready for review September 25, 2021 16:50
@levouh
Copy link
Contributor Author

levouh commented Sep 25, 2021

Let me know how that looks @jose-elias-alvarez.

@jose-elias-alvarez
Copy link
Owner

Looks great, thank you for working on this (and for the improvements to the documentation)!

@jose-elias-alvarez jose-elias-alvarez merged commit 204b4e6 into jose-elias-alvarez:main Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants