Repository: Canop/bacon (language: rust)

Issue:
Pass the command for a job as an argument to bacon
This is by no means a need but I've look and can't see that anyone asked before. I wanted to know if supplying the command job details to run on the command line would be something you'd be amenable to. So you could run one off jobs for example (I know cargo test has a built in job just didn't have a better idea for the example).

```sh
bacon -c "cargo test"
```

would be equivalent to having a `bacon.toml` like this:

```toml
[jobs.one-off]
command = ["cargo", "test"]
```

and then running

```sh
bacon -j one-off
```


Discussion / hints:
Problem is there are usually more parameters for a job, and it's only growing with bacon being used on non cargo tools.
Sorry I did think about there being many parameters and 100% forgot to include how I feel that could be addressed along with why I think this feature is useful in the first place.

# Many job parameters

I considered 3 solutions to this problem.  I'll list them in order of my preference but you have a better idea of the big picture than I do so feel free to not pick number 1 or none if you see it's a bad idea in a way I don't. (I'm also willing to try to implement it if time is the issue because I think it adds sufficient value, more on the value in the motivation.)

## 1 - Pass full job as one argument in json

Instead of adding a flag for each parameter just allow the user to pass all of them as a json string. I imagine it working by using `-C` to pass a full job config. And `-c` would be to pass just the command and use the defaults for the rest. Note the specific text format is not important just used json to explain as that works well all on one line. So for example  you could call it with just the command 

```sh
bacon -c "cargo test"
```

or if you need to change other settings

```sh
bacon -C '{"command": ["cargo", "test"], "need_stdout": true}'
```

For implementing the feature, I'm imagining that the string could be deserialized as a job assuming that's what happens right now with the toml. But either way what ever is done now for the toml try to do something similar.

## 2 - Only support common parameters

Alternatively only popular parameters could be supported and if you need to go outside of that then you need to create a bacon.toml as is needed now.

## 3 - Only support command with defaults

This is the simplest but also most limiting. And I suspect it would actually generate more complaints than value.

# Why do this at all (MOTIVATION)

It's much easier to share a command to run than to give instructions on how to insert a job into a bacon.toml if you are trying to help someone remotely. I also often find myself using one off tasks like just to test a command and right now that means creating a job to test it. It's easier now with the hot config reload but would be easier still with being able to just supply the job at the command line where I don't need to separate each argument in quotes and commas.
Is this something worth pursuing?
I don't know. I'm afraid it would be too limiting and end up in the addition of tons of launch parameters. I had a similar experience in my current project at work, where I finally had to drop pure command-line launches and keep only file based tasks.

The job passed as JSON as you propose in 2 is interesting but typing JSON by hand on the command line is far from convenient or easy.
BTW I'd like other bacon users to give their opinion.
Agreed. Would be great to get feedback from more users. I agree about json not being convenient to type and I agree the multiple launch parameters doesn't seem viable. I'm concerned that the json ones while easier to share would still be very long but I just completed [Shuttle's Christmas Code Hunt](https://console.shuttle.dev/shuttlings/cch24) and they shared long commands for curl and actually it wasn't that bad.

This is an example from the challenge for Day 5, the top part is the command and the bottom is the expected output

![image](https://github.com/user-attachments/assets/850fde0d-0ee6-4d3a-b5b2-a0b9a0cc8705)

My thoughts are:

* This would be nice to have
* A `--command/-c` flag with a single string argument for the command to run with all args is certainly a reasonable way to set this up. Alternatively a "run" or "on-off" subcommand that took the command arguments as final SPLAT argument (past a `--` separator) would also work and be welcome by shell gurus.
* JSON blobs in shell args is a non-starter in my book. Gross.
* All the *other* stuff that could/would be needed should be regular arguments. I would argue that with or without this one off job feature, any and every option that can be set for a job in the config should be overridable with CLI args anyway. Yes this means quite a few arguments. No I don't think that's a bad thing.
Let's be concrete to better judge, starting with the current list of mission arguments.

The arguments of a job are defined here: https://github.com/Canop/bacon/blob/main/src/jobs/job.rs

Ignoring the arguments that are already present as launch arguments, here's the list of what's missing if we want to define any job at the command line, with my opinion regarding their necessity level.

* allow_failures: essential
* allow_warnings: essential
* analyzer: essential
* apply_gitignore: optional
* background: essential
* command: essential
* default_watch: nice to have
* env: optional (we can pass env vars otherwise)
* expand_env_vars: optional
* extraneous_args: not needed
* ignore: optional
* ignored_lines: probably optional
* kill: essential
* need_stdout: essential
* on_change_strategy: essential for long running programs
* on_success: not necessary for cmd call

I don't think I want to dilute existing launch arguments with such profusion and I really don't want to have to tell users "this one liner doesn't work because you didn't specify `background=false` or `analyzer=nextest` which you can't because it's not a launch argument").
Passing commands with the equivalent of `--data` looks like a good idea both to avoid 20 more launch arguments and to allow more complex setups, eg with several jobs. I would probably call that `--conf-toml` to prepare for the support of eg Hjson and JSON.

Maybe we could also pass the config file on stdin.
@alerque Right now, "the command arguments as final SPLAT argument (past a -- separator)" are given to the executed command
Another option I hadn't thought of (and am not sure it's a great idea) is to derive clap::Args for the struct that stores the jobs. That way it will automatically create flags for each of the fields and put them all under a sub command. I tried to copy the actual relevant types from bacon, [Args](https://github.com/Canop/bacon/blob/704dff7cd95b5f2743d3d3af3845b996dbaa52f7/src/conf/args.rs#L34) and [Jobs](https://github.com/Canop/bacon/blob/704dff7cd95b5f2743d3d3af3845b996dbaa52f7/src/jobs/job.rs#L9) but they were too long and I also didn't want to invest too much time into just showing the idea as it may not even be a good fit. I also enabled the default help so I could show it in the example but I suspect that won't work for bacon in practice so there might be even more overhead to going with this approach.

# Output of help on top level binary

```
$ cargo run -- -h
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/bacon -h`
Launch arguments

Usage: bacon [OPTIONS] [COMMAND]

Commands:
  inline  Pass the job to be run on the command line
  help    Print this message or the help of the given subcommand(s)

Options:
      --prefs    Print the path to the prefs file, create it if it doesn't exist
  -h, --help     Print help
  -V, --version  Print version
```

# Output of help on the subcommand

```
$ cargo run -- inline -h
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/bacon inline -h`
Pass the job to be run on the command line

Usage: bacon inline [OPTIONS]

Options:
      --allow-failures       Whether to consider that we can have a success when we have test failures
      --allow-warnings       Whether to consider that we can have a success when we have warnings. This is
                             especially useful for "cargo run" jobs
      --analyzer <ANALYZER>  The analyzer interpreting the output of the command, the standard cargo dedicated one
                             if not provided
  -h, --help                 Print help
```

# Code used (plus clap in the cargo toml)

```rust
use clap::{command, Args as ClapArgs, Parser, Subcommand};

/// Launch arguments
#[derive(Debug, Parser)]
#[command(
    author,
    about,
    version,
    // disable_version_flag = true,
    // disable_help_flag = true
)]
pub struct Args {
    /// Print the path to the prefs file, create it if it doesn't exist
    #[clap(long)]
    pub prefs: bool,

    #[command(subcommand)]
    command: Option<Command>,
}

#[derive(Debug, Subcommand)]
#[command()]
enum Command {
    /// Pass the job to be run on the command line
    #[command()]
    Inline(Job),
}

/// One of the possible job that bacon can run
#[derive(ClapArgs, Debug, Clone)]
pub struct Job {
    /// Whether to consider that we can have a success
    /// when we have test failures
    #[clap(long)]
    pub allow_failures: bool,

    /// Whether to consider that we can have a success
    /// when we have warnings. This is especially useful
    /// for "cargo run" jobs
    #[clap(long)]
    pub allow_warnings: bool,

    /// The analyzer interpreting the output of the command, the
    /// standard cargo dedicated one if not provided
    #[clap(long)]
    pub analyzer: Option<String>,
}

fn main() {
    let args = Args::parse();
    dbg!(args);
}
```
IMO the cleanest solution now is the `--data` one. It's only one parameter and would only require a few added lines of code to support. The main questions are "is this convenient?", "is this useful?", and "how should this launch parameter be called?".

I suppose we should add it as an experiment to answer at least the 2 first questions.
> IMO the cleanest solution now is the `--data` one. It's only one parameter and would only require a few added lines of code to support.

And importantly it doesn't require more code to support it as features get added. It's the one with the lowest maintenance burden which IMO is very important. And honestly from my point of view as a user it's the easiest to understand. I can reuse what I would have put in the toml file and don't need to consult the help for each field I want to add.

> The main questions are "is this convenient?",

It's not the most convenient because it requires longer commands than the feature flag option but that said I think the maintenance trade off makes it worthwhile and will be easier to read than the feature flag option. So IMO it's the most balanced in terms of convenience, easier to read and easier to maintain. Just harder to type but it's typed once and maintained and read repeatedly.

> "is this useful?", and 

Personally I think it is very useful. I have to write up [some instructions](https://github.com/shuttle-hq/shuttle/issues/1944) on how to use bacon and it's much easier to give one command even if it's long compared to instructions on explaining how to setup a bacon toml, how to set the command to run, how to set the needed fields, then how to run the job.

> "how should this launch parameter be called?".
 
This I'm not sure about, personally I like that "data" is short but I wouldn't guess that's what it does. I like `inline_toml` or something like that but that's very long.

> I suppose we should add it as an experiment to answer at least the 2 first questions.

I think that's a great idea but I think we should not include it in the default install and rather add an experimental feature flag so users can opt into it when installing but it's not the default so we can remove it without unnecessary breakage.
I'll add it, then.

Regarding the name, I'd like to convey that it's another config element, equivalent to the various prefs.toml and bacon.toml files, and that it's in toml so that bacon can more easily support other configuration languages in the future.
Yes, I agree. To confirm it would have the highest priority so just above the bacon.toml?
Yes, just before other command line arguments
> Yes, just before other command line arguments

I can't figure out what difference the order of the arguments makes? Is it a restriction imposed by Clap, I though unless arguments were positional it didn't matter.
I'm speaking of priorities: I would apply the settings of the `--data` thing before applying the other launch arguments (which would then override it)

oh, I see. Yes that makes sense. So it would be treated like a higher priority bacon.toml that gets overridden on the command line. Ok that makes sense.

Fix patch:
diff --git a/src/conf/args.rs b/src/conf/args.rs
index b18256c..0824fb4 100644
--- a/src/conf/args.rs
+++ b/src/conf/args.rs
@@ -126,6 +126,10 @@ pub struct Args {
     #[clap(long, value_name = "project")]
     pub project: Option<String>,
 
+    /// Configuration passed as a TOML string
+    #[clap(long)]
+    pub config_toml: Option<String>,
+
     #[clap()]
     /// What to do: either a job, or a path, or both
     pub args: Vec<String>,
diff --git a/src/conf/settings.rs b/src/conf/settings.rs
index 8881baf..027869b 100644
--- a/src/conf/settings.rs
+++ b/src/conf/settings.rs
@@ -88,6 +88,7 @@ impl Settings {
     /// * the package level `bacon.toml` file in package-root/.bacon.toml
     /// * the package level `bacon.toml` file in package-root/.config/.bacon.toml
     /// * the file whose path is in environment variable `BACON_CONFIG`
+    /// * the content of the `--config-toml` argument
     /// * args given as arguments, coming from the cli call
     pub fn read(
         args: &Args,
@@ -122,6 +123,12 @@ impl Settings {
             }
         }
 
+        if let Some(toml) = &args.config_toml {
+            let config = toml::from_str(toml)?;
+            info!("config loaded from --config-toml: {:#?}", &config);
+            settings.apply_config(&config);
+        }
+
         settings.apply_args(args);
         settings.check()?;
         info!("settings: {:#?}", &settings);
diff --git a/website/docs/community/bacon-dev.md b/website/docs/community/bacon-dev.md
index d199492..3298294 100644
--- a/website/docs/community/bacon-dev.md
+++ b/website/docs/community/bacon-dev.md
@@ -41,3 +41,5 @@ If bacon didn't understand correctly the output of a cargo tool, it may also be
 
 If you think you might help, as a tester or coder, you're welcome, but please read [Contributing to my FOSS projects](https://dystroy.org/blog/contributing/) before starting a PR.
 
+
+**Don't open a PR without discussing the design before**, either in the chat or in an issue, unless you're just fixing a typo. Coding is the easy part. Determining the exact requirement and how we want it to be done is the hard part. This is especially important if you plan to add a dependency or to change the visible parts, eg the launch arguments.
diff --git a/website/docs/config.md b/website/docs/config.md
index e159916..74eafbd 100644
--- a/website/docs/config.md
+++ b/website/docs/config.md
@@ -17,6 +17,7 @@ Bacon loads in order:
 * the `bacon.toml` file in `package-root/`
 * the `bacon.toml` file in `package-root/.config/`
 * the file whose path is in environment variable `BACON_CONFIG`
+* the content of the `--config-toml` argument
 
 Each configuration file overrides the properties of previously loaded ones.
 
diff --git a/website/docs/cookbook.md b/website/docs/cookbook.md
index 603fa95..8429c08 100644
--- a/website/docs/cookbook.md
+++ b/website/docs/cookbook.md
@@ -150,5 +150,28 @@ You would call it with
 bacon ex -- example4578
 ```
 
+# Bacon CLI snippets
 
+You may share a command-line snippet without requiring a bacon.toml file, using the `--config-toml` argument:
+
+```bash
+bacon -j cli-test --config-toml '
+[jobs.cli-test]
+command = [
+  "sh",
+  "-c",
+  "echo \"hello $(date +%H-%M-%S)\"; cargo run",
+]
+need_stdout = true
+allow_warnings = true
+background = false
+on_change_strategy = "kill_then_restart"
+kill = ["pkill", "-TERM", "-P"]'
+```
+
+Notes:
+
+* wrap the inline configuration item in single quotes so that you may use double-quotes inside
+* this configuration is simply added to the other ones and it may refer to them
+* if you add a job this way and want it executed, either define it as `default_job` in the same inline TOML or use the `--job`/`-j` argument as in the example above
 


Test patch: