Allow to set the command programatically

[f4z3r]

Fixes #42

It's a good idea to open an issue first for discussion.

• Tests pass
• Appropriate changes to README are included in PR

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[f4z3r]

Hi @jackfranklin, is this still maintained? I am just asking because I currently run a fork to support this feature. I would really like to use this plugin to reduce the amount of custom code I manage. But if it is no longer maintained I might switch to something fully custom either way to ensure everything is up to date 😄

[jackfranklin]

Hey sorry, I completely missed this. I will take a look.

[jackfranklin]

Some thoughts:

1. This approach still causes the blocking vim.ui.input to appear, just with a value pre-populated. Is that what you want? Or really, do you want a way to set it without going through any UI layer at all? These feel like two different things; either a {placeholder=} label for the input, or a new method to set the cmd with no UI at all.
2. M.run(clear_existing_cmd, cmd) will only check cmd` if there is no stored command. This is probably not what you want and feels like a bug. Also I don't love that API where the cmd isn't first. Needs a rethink, I think.

There are some further thoughts I have but I think the two above are the main concerns, I would like to think more about the API we expose here.

Mainly: do you want prepopulated input fields, or just a programmatic solution that avoids the UI? I think we need to decide that, before we figure how best to support it.

[f4z3r]

Hi @jackfranklin

No worries, thanks for the quick reply. Regarding your points:

1. I personally would prefer to be able to control the command fully programmatically, thus without having to confirm it in the UI. I made this change as it was the smallest change that achieved more or less what I need. At the moment I don't mind the extra "pressing enter" and it might have a value down the road if one wants to change the pre-populated command (not my current use case).
2. Yes, I kind of did this on purpose. The reason is that if cmd always overwrites the existing command, the first argument clear_existing_command essentially loses its meaning when cmd is given. I absolutely agree that having cmd as the second argument is not ideal.

So long story short: I would be more interested in a way to control the executor fully programmatically without any UI interactions needed.

[jackfranklin]

OK, I think in that case we need some new API for this.

executor.cmmands.run_with_new_command() -- prompts for new input

Is what we currently have. What if we expose executor.api. for your methods?

executor.api.set_command('npm test')
executor.api.run_with_new_command('npm test') -- sets and runs
executor.api.run() -- throws if no command is set

[f4z3r]

Yup, exposing executor.api seems like the cleanest way. I will update the PR tomorrow morning 👍🏽 Thanks for the feedback.

[f4z3r]

@jackfranklin I realized that if we expose some of the "internal" API of the Executor, most functionality is already there. I re-exposed those functions rather than modifying run (or re-exposing another function under the run name) to throw in case of missing command. What do you think?

The only thing at the moment is that run_task with no argument when no task is set will simply return. There is no feedback that nothing happened (no throwing, no notification, etc). How would you have this handled? I guess throwing is better feedback for a programmatic API, as the client can then choose what to do. Would you prefer this to be handled via a return value or an actual error throwing?

[jackfranklin]

Sorry I missed this! Your PR looks good.

Let's make run_task throw if there is no command to run - but only in the API one. I don't want to make it an error if you accidentally trigger it in the Vim UI and then hit enter with an empty input. It's fine that nothing happens then.

So the public API function should wrap the existing API but check for a cmd (as an arg or stored) and error if it doesn't have one.