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

Allow command predicates to manage parameters, allow overwriting commands #1098

Merged
merged 4 commits into from Aug 16, 2022

Conversation

Guldoman
Copy link
Member

Command predicates can now return more values after the command "validity" boolean. Those parameters are then passed to the actual commands.
Command predicates now also receive the parameters passed to command.perform.
This allows command predicates to manipulate or create from scratch the parameters that are passed to the actual commands.

command.add(function(x, y)
    return true, x + 1, y + 1
  end, {
  ["cmd:do-thing"] = function(x, y)
      print(x, y)
    end
  }
)

command.perform("cmd:do-thing", 1, 2) -- prints 2, 3

This also has the advantage of allowing to do heavy computations that are needed both by the predicate and the actual command only once.

command.add(function(a, b)
    local result = heavy_computation(a, b)
    return result.status == "OK", result
  end, {
  ["cmd:do-heavy-thing"] = function(heavy_result)
      do_thing_with(heavy_result)
    end
  }
)

command.perform("cmd:do-heavy-thing", a, b)

If a command predicate doesn't return any additional parameters, the original ones are passed through to the actual command.

command.add(function()
    return true
  end, {
  ["cmd:do-thing2"] = function(x, y)
      print(x, y)
    end
  }
)

command.perform("cmd:do-thing2", 1, 2) -- prints 1, 2

Now string and table predicates also return the active view. Additional parameters are also passed through.

local DocView = require "core.docview"

command.add("core.docview!", {
  ["cmd:do-thing-with-docview"] = function(dv, x, y)
      print(dv:is(DocView), x, y)
    end
  }
)

command.perform("cmd:do-thing-with-docview", 1, 2) -- prints true, 1, 2

Now when command.get_all_valid is called, each predicate is memoized to avoid recomputing it for each command that uses it.
This could be an important optimization for predicates that need heavy computations that are also used for multiple commands.


It's now possible to overwrite commands without errors.
While the assertion might have been useful to debug erroneous command overwrites, it also caused issues when saving the user module with commands defined inside it, as it resulted in the user-defined commands trying to overwrite themselves and failing.


As I changed many commands to use the new parameter passing, testing is needed as I might have missed something.

@jgmdev
Copy link
Member

jgmdev commented Aug 16, 2022

Are these missing dv replacements for doc() on purpose?

https://github.com/Guldoman/lite-xl/blob/PR_command_predicate_params/data/core/commands/doc.lua#L596-L619

When a command is performed with parameters, those are now passed to the
predicate.
The predicate can then return, after the validity boolean, any other
value that will then be passed to the actual command.
If the predicate only returns the validity boolean, the original
parameters are passed through to the actual command.

This allows predicates to manipulate the received parameters, and allows
them to pass the result of an expensive computation to the actual
command, which won't have to recalculate it.

String and table predicates will now also return `core.active_view`.
This caused issues when saving the user module with commands defined
inside it, as it resulted in the user-defined commands trying to
overwrite themselves and failing.
@Guldoman
Copy link
Member Author

Are these missing dv replacements for doc() on purpose?

https://github.com/Guldoman/lite-xl/blob/PR_command_predicate_params/data/core/commands/doc.lua#L596-L619

Totally forgot about them.

@jgmdev
Copy link
Member

jgmdev commented Aug 16, 2022

Tested and things seem to be working normally, merging.

@jgmdev jgmdev merged commit 76f71b2 into lite-xl:master Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants