Repository: Nukesor/pueue (language: rust)

Issue:
Enhance `pueue edit` UX
Hello, and thank you for this very useful program!

I often forget to pass some options when adding new tasks,
and find myself needing to edit those, often in batch.

The current `edit` sub-command opens an editor for each value to edit,
which is quite cumbersome when just wanting to replace a value,
especially for multiple tasks at once.

Therefore, I think it would be nice for the `edit` sub-command to:
- accept mulitple task IDs at once, and
- accept replacement values directly from the CLI,
  to be applied to all the specified tasks at once.


__Current help text of the `edit` sub-command:__

```
❯ pueue edit --help
Edit the command, path, label, or priority of a stashed or queued task.
By default only the command is edited.
Multiple properties can be added in one go.

Usage: pueue edit [OPTIONS] <TASK_ID>

Arguments:
  <TASK_ID>  The task's id

Options:
  -c, --command   Edit the task's command
  -p, --path      Edit the task's path
  -l, --label     Edit the task's label
  -o, --priority  Edit the task's priority
  -h, --help      Print help
```


__Target help text with the suggested additions:__

```
❯ pueue edit --help
Edit the command, path, label, or priority of a stashed or queued task(s).
Values specified in options are used as replacements.
When no option is specified, EDITOR is opened.

Usage: pueue edit [OPTIONS] [TASK_IDS]...

Arguments:
  [TASK_IDS]...  The task(s) id

Options:
  -c, --command <COMMAND>    Edit the task(s)' command
  -p, --path <PATH>          Edit the task(s)' path
  -l, --label <LABEL>        Edit the task(s)' label
  -o, --priority <PRIORITY>  Edit the task(s)' priority
  -h, --help                 Print help
```

When no replacement option is supplied on the CLI,
the default EDITOR can be used to edit the properties.

But instead of opening the EDITOR for each value for each task individually,
this could be done in a combined way in a single YAML or TOML file.

For example, running `pueue edit 0 1` would open an EDITOR with:

```toml
[0]
command = "echo this is my first command"
path = "/home/user"
label =
priority = 0

[1]
command = "echo this is my second command"
path = "/home/user/whatever"
label =
priority = 5
```

Since the current `edit` sub-command is quite interactive,
these modificatinos shouldn't break existing automated user scripts.


Discussion / hints:
Thanks for the detailed feature request :)

I already thought about this some time ago, over [here](https://github.com/Nukesor/pueue/issues/449).

I would like to take some time to re-evaluate on which approach is the most user-friendly, but this'll have to wait for a bit :)


So, I thought about this for quite some time and this problem is pretty tricky.

As users can theoretically start any valid shell syntax in any shell they like (as the shell is configurable), we have practically no guarantees on how the input will look like.

To keep this convenient, we must allow multi-line editing. Strings with "\n" inside of it aren't an option.
No structured data format i know of can properly handle this. All of them have some kind of reserved characters or other constraints that make them unfeasible.

So, after some discussions with friends and other people, we came up with the idea to instead of creating a temporary file, to rather create a temporary directory with a file for each property to add.

All modern editors have directory/file tree support, so users can simply select the files they're interested in and edit without any file format constraints.

I really like this solution as it is **very** simple and outsources the UI issue to the user's editor, which is a highly specialized tool for editing multiple files.
This could then be expanded for multiple tasks, where we create subdirectories for each task to edit with the ID being the  name of the subdirectory.
Hi thanks for the new feature!

Here is my naive thoughts: Maybe we can let user edit a YAML file. For example,

```
- command: "echo this is my first command"
  path: "/home/user"
  label: 
  priority: 0
- command: "echo this is my second command"
  path: "/home/user"
  label: 
  priority: 0
```

Or in TOML:

```
[[commands]]
command = "echo this is my first command"
path = "/home/user"
label = ""
priority = 0

[[commands]]
command = "echo this is my second command"
path = "/home/user"
label = ""
priority = 0
```

Pros:

* Everything in one file, thus easy to quickly modify. For example, I use vim, and can *very* quickly move these tasks around (as they are just several lines of "code"). On the other hand, if it is a folder (or multi folders), then it is not very easy.

Extension:

* It would be great to support more "declarative" ways. For example, if I add several new lines, then that will be treated as adding a task. If I delete some lines, then that is parsed as removing a task.

If we want to allow multiline edit without "\n" in it: YAML has nice multiline string support https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-in-yaml-over-multiple-lines. TOML also have https://toml.io/en/.

Using yaml or toml was one of the very first ideas. I actually like yamls `|` block quite a bit but there's this annoying thing that there's currently no well-maintained serde yaml library out there that supports that formatting option (at least none I know of). So that option dropped out due to a rather trivial technical reason.


Regardless of that, there're multiple pros and cons for both sides.

# File based structure

## Pros
- Allows any input (hard to break, very robust)
- Very clear separation
- Intuitive
- Works for hundreds of tasks without issues

## Cons

- Your Editor should have a file tree for this to be useful.
- Is probably a bit slower for few tasks

# Structured Data

## Pros
- Single file, simple to navigate
- Nicer for editing few tasks

## Cons
- Users must know about the file format, not everybody is familiar with toml/yaml.
- We allow full shell syntax with configurable shells as `command` input. Users must have detailed knowledge about reserved characters of the used format if they write complex scripts, otherwise they might break their commands.


---

A friend of mine proposed to make this configurable and allow both solutions. I tend to go towards that approach as I see that the structured data solution is nicer for some users and is easier to use when your editor doesn't have a file tree.
Thanks for the reply! I personally love the structured data approach, so my thought below are opinionated :)

> So that option dropped out due to a rather trivial technical reason.

Thus curious whether TOML has one for that

> not everybody is familiar with toml/yaml

I guess it is intuitive enough that it is not hard for users to realize the format.

> Users must have detailed knowledge about reserved characters of the used format if they write complex scripts, otherwise they might break their commands.

https://toml.io/en/ seems to support "literal string" like below. Thus, seems there are *zero* reserved characters (except for three consequtive quotes `'''`)

```
re = '''I [dw]on't need \d{2} apples'''
lines = '''
The first newline is
trimmed in raw strings.
All other whitespace
is preserved.
'''
```

> A friend of mine proposed to make this configurable and allow both solutions. I tend to go towards that approach as I see that the structured data solution is nicer for some users and is easier to use when your editor doesn't have a file tree.

Looking forward to it!

I'm going to build this, but I just want to drive home my point of view.

> I guess it is intuitive enough that it is not hard for users to realize the format.

Pueue is being used by quite a few people in the science sector, specifically machine learning. Those aren't necessarily ver tech savy.

> https://toml.io/en/ seems to support "literal string" like below. Thus, seems there are zero reserved characters (except for three consequtive quotes ''')

For this, users need to know about `'''`, they need to be fairly familiar with toml/yaml and know that they can simply use that to have a long string in which they don't have to worry about character escaping or multi line shennanigans.
It may seem very intuitive and easy for you, but it won't be intuitive for everyone, especially for people that haven't worked with toml before (i.e. People that don't use Python or Rust).

As I said, I plan to support both approaches, I just want to make clear that it isn't that easy.
I see, good point!

Btw, people in machine learning are often using python nowadays (I am also doing machine learning). But yes, often they do not work with TOML (maybe pyproject.toml is often the only toml they will see?).

>  know that they can simply use that to have a long string in which they don't have to worry about character escaping or multi line shennanigans

For this, not sure whether we can generate toml with comments like:

```
# you can have multiline and no worries about escaping inside triple quotes
command: '''
the current command
'''
I'm open to contributions on this one :)

If anyone wants to work on this, feel free to go ahead. I currently have quite a bit on my plate and don't know when I'll get to it :) 

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a024641..c4ea0e57 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -74,6 +74,7 @@ Upon updating Pueue and restarting the daemon, the previous state will be wiped,
 - Change default log level from error to warning [#562](https://github.com/Nukesor/pueue/issues/562).
 - Bumped MSRV to 1.70.
 - **Breaking**: Redesigned task editing process [#553](https://github.com/Nukesor/pueue/issues/553).
+  Pueue now allows editing all properties a task in one editor session. There're two modes to do so: `toml` and `files`.
 
 ### Add
 
diff --git a/Cargo.lock b/Cargo.lock
index bceb5c39..3a160492 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -585,7 +585,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "33d852cb9b869c2a9b3df2f71a3074817f01e1844f839a144f5fcef059a4eb5d"
 dependencies = [
  "libc",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -1272,6 +1272,7 @@ dependencies = [
  "tempfile",
  "test-log",
  "tokio",
+ "toml",
  "whoami",
  "windows",
  "windows-service",
@@ -1532,7 +1533,7 @@ dependencies = [
  "errno",
  "libc",
  "linux-raw-sys",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -1642,6 +1643,15 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "serde_spanned"
+version = "0.6.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "87607cb1398ed59d48732e575a4c28a7a8ebf2454b964fe3f224f2afc07909e1"
+dependencies = [
+ "serde",
+]
+
 [[package]]
 name = "serde_yaml"
 version = "0.9.34+deprecated"
@@ -1820,7 +1830,7 @@ dependencies = [
  "getrandom",
  "once_cell",
  "rustix",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -1980,11 +1990,26 @@ dependencies = [
  "tokio",
 ]
 
+[[package]]
+name = "toml"
+version = "0.8.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1ed1f98e3fdc28d6d910e6737ae6ab1a93bf1985935a1193e68f93eeb68d24e"
+dependencies = [
+ "serde",
+ "serde_spanned",
+ "toml_datetime",
+ "toml_edit",
+]
+
 [[package]]
 name = "toml_datetime"
 version = "0.6.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41"
+dependencies = [
+ "serde",
+]
 
 [[package]]
 name = "toml_edit"
@@ -1993,6 +2018,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4ae48d6208a266e853d946088ed816055e556cc6028c5e8e2b84d9fa5dd7c7f5"
 dependencies = [
  "indexmap",
+ "serde",
+ "serde_spanned",
  "toml_datetime",
  "winnow",
 ]
@@ -2233,7 +2260,7 @@ version = "0.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb"
 dependencies = [
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
diff --git a/Cargo.toml b/Cargo.toml
index d9d177bf..ddf4e8ff 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,8 +12,6 @@ repository = "https://github.com/nukesor/pueue"
 rust-version = "1.70"
 
 [workspace.dependencies]
-# Chrono version is hard pinned to a specific version.
-# See https://github.com/Nukesor/pueue/issues/534
 anyhow = "1"
 better-panic = "0.3"
 chrono = { version = "0.4", features = ["serde"] }
diff --git a/pueue/Cargo.toml b/pueue/Cargo.toml
index 1cb7c45a..b914f825 100644
--- a/pueue/Cargo.toml
+++ b/pueue/Cargo.toml
@@ -37,6 +37,7 @@ snap.workspace = true
 strum.workspace = true
 tempfile = "3"
 tokio.workspace = true
+toml = "0.8"
 
 [dev-dependencies]
 anyhow.workspace = true
diff --git a/pueue/src/client/commands/edit.rs b/pueue/src/client/commands/edit.rs
index e69136eb..e6214ced 100644
--- a/pueue/src/client/commands/edit.rs
+++ b/pueue/src/client/commands/edit.rs
@@ -1,3 +1,4 @@
+use std::collections::BTreeMap;
 use std::env;
 use std::fs::{create_dir, read_to_string, File};
 use std::io::Write;
@@ -33,34 +34,37 @@ pub async fn edit(
     // In case we don't receive an EditResponse, something went wrong.
     // Return the response to the parent function and let the client handle it
     // by the generic message handler.
-    let Message::EditResponse(mut editable_tasks) = init_response else {
+    let Message::EditResponse(editable_tasks) = init_response else {
         return Ok(init_response);
     };
 
-    let edit_result = edit_tasks(settings, &mut editable_tasks);
+    let task_ids: Vec<usize> = editable_tasks.iter().map(|task| task.id).collect();
+    let result = edit_tasks(settings, editable_tasks);
 
     // Any error while editing will result in the client aborting the editing process.
     // However, as the daemon moves tasks that're edited into the `Locked` state, we cannot simply
     // exit the client. We rather have to notify the daemon that the editing process was interrupted.
     // In the following, we notify the daemon of any errors, so it can restore the tasks to
     // their previous state.
-    if let Err(error) = edit_result {
-        eprintln!("Encountered an error while editing. Trying to restore the task's status.");
-        // Notify the daemon that something went wrong.
-        let task_ids = editable_tasks.iter().map(|task| task.id).collect();
-        let edit_message = Message::EditRestore(task_ids);
-        send_message(edit_message, stream).await?;
-
-        let response = receive_message(stream).await?;
-        match response {
-            Message::Failure(message) | Message::Success(message) => {
-                eprintln!("{message}");
-            }
-            _ => eprintln!("Received unknown response: {response:?}"),
-        };
-
-        return Err(error);
-    }
+    let editable_tasks = match result {
+        Ok(editable_tasks) => editable_tasks,
+        Err(error) => {
+            eprintln!("Encountered an error while editing. Trying to restore the task's status.");
+            // Notify the daemon that something went wrong.
+            let edit_message = Message::EditRestore(task_ids);
+            send_message(edit_message, stream).await?;
+
+            let response = receive_message(stream).await?;
+            match response {
+                Message::Failure(message) | Message::Success(message) => {
+                    eprintln!("{message}");
+                }
+                _ => eprintln!("Received unknown response: {response:?}"),
+            };
+
+            return Err(error);
+        }
+    };
 
     // Create a new message with the edited properties.
     send_message(Message::Edit(editable_tasks), stream).await?;
@@ -68,11 +72,75 @@ pub async fn edit(
     Ok(receive_message(stream).await?)
 }
 
-pub fn edit_tasks(settings: &Settings, editable_tasks: &mut [EditableTask]) -> Result<()> {
+/// This is a small generic wrapper around the editing logic.
+///
+/// There're two different editing modes in Pueue, one file based and on toml based.
+/// Call the respective function based on the editing mode.
+pub fn edit_tasks(
+    settings: &Settings,
+    editable_tasks: Vec<EditableTask>,
+) -> Result<Vec<EditableTask>> {
     // Create the temporary directory that'll be used for all edits.
     let temp_dir = tempdir().context("Failed to create temporary directory for edtiting.")?;
     let temp_dir_path = temp_dir.path();
 
+    match settings.client.edit_mode {
+        pueue_lib::settings::EditMode::Toml => {
+            edit_tasks_with_toml(settings, editable_tasks, temp_dir_path)
+        }
+        pueue_lib::settings::EditMode::Files => {
+            edit_tasks_with_folder(settings, editable_tasks, temp_dir_path)
+        }
+    }
+}
+
+/// This editing mode creates a temporary folder that contains a single `tasks.toml` file.
+///
+/// This file contains all tasks to be edited with their respective properties.
+/// While this is very convenient, users must make sure to not malform the content and respect toml
+/// based escaping as not doing so could lead to deserialization errors or broken/misbehaving
+/// task commands.
+pub fn edit_tasks_with_toml(
+    settings: &Settings,
+    editable_tasks: Vec<EditableTask>,
+    temp_dir_path: &Path,
+) -> Result<Vec<EditableTask>> {
+    // Convert to map for nicer representation and serialize to toml.
+    // The keys of the map must be strings for toml to work.
+    let map: BTreeMap<String, EditableTask> = BTreeMap::from_iter(
+        editable_tasks
+            .into_iter()
+            .map(|task| (task.id.to_string(), task)),
+    );
+    let toml = toml::to_string(&map)
+        .map_err(|err| Error::Generic(format!("\nFailed to serialize tasks to toml:\n{err}")))?;
+    let temp_file_path = temp_dir_path.join("tasks.toml");
+
+    // Write the file to disk and open it with the editor.
+    std::fs::write(&temp_file_path, toml).map_err(|err| {
+        Error::IoPathError(temp_file_path.clone(), "creating temporary file", err)
+    })?;
+    run_editor(settings, &temp_file_path)?;
+
+    // Read the data back from disk into the map and deserialize it back into a map.
+    let content = read_to_string(&temp_file_path)
+        .map_err(|err| Error::IoPathError(temp_file_path.clone(), "reading temporary file", err))?;
+    let map: BTreeMap<String, EditableTask> = toml::from_str(&content)
+        .map_err(|err| Error::Generic(format!("\nFailed to deserialize tasks to toml:\n{err}")))?;
+
+    Ok(map.into_values().collect())
+}
+
+/// This editing mode creates a temporary folder in which one subfolder is created for each task
+/// that should be edited.
+/// Those task folders then contain a single file for each of the task's editable properties.
+/// This approach allows one to edit properties without having to worry about potential file
+/// formats or other shennanigans.
+pub fn edit_tasks_with_folder(
+    settings: &Settings,
+    mut editable_tasks: Vec<EditableTask>,
+    temp_dir_path: &Path,
+) -> Result<Vec<EditableTask>> {
     for task in editable_tasks.iter() {
         task.create_temp_dir(temp_dir_path)?
     }
@@ -84,7 +152,7 @@ pub fn edit_tasks(settings: &Settings, editable_tasks: &mut [EditableTask]) -> R
         task.read_temp_dir(temp_dir_path)?
     }
 
-    Ok(())
+    Ok(editable_tasks)
 }
 
 /// Open the folder that contains all files for editing in the user's `$EDITOR`.
diff --git a/pueue/src/client/commands/restart.rs b/pueue/src/client/commands/restart.rs
index 72e4c54d..c9083fc1 100644
--- a/pueue/src/client/commands/restart.rs
+++ b/pueue/src/client/commands/restart.rs
@@ -101,7 +101,7 @@ pub async fn restart(
     // If the tasks should be edited, edit them in one go.
     if edit {
         let mut editable_tasks: Vec<EditableTask> = tasks.iter().map(EditableTask::from).collect();
-        edit_tasks(settings, &mut editable_tasks)?;
+        editable_tasks = edit_tasks(settings, editable_tasks)?;
 
         // Now merge the edited properties back into the tasks.
         // We simply zip the task and editable task vectors, as we know that they have the same
diff --git a/pueue_lib/src/settings.rs b/pueue_lib/src/settings.rs
index db495dce..d909eaea 100644
--- a/pueue_lib/src/settings.rs
+++ b/pueue_lib/src/settings.rs
@@ -82,6 +82,16 @@ pub struct Shared {
     pub shared_secret_path: Option<PathBuf>,
 }
 
+/// The mode in which the client should edit tasks.
+#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize, Default)]
+pub enum EditMode {
+    /// Edit by having one large file with all tasks to be edited inside at the same time
+    #[default]
+    Toml,
+    /// Edit by creating a folder for each task to be edited, where each property is a single file.
+    Files,
+}
+
 /// All settings which are used by the client
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
 pub struct Client {
@@ -97,6 +107,9 @@ pub struct Client {
     /// Whether the client should show a confirmation question on potential dangerous actions.
     #[serde(default = "Default::default")]
     pub show_confirmation_questions: bool,
+    /// Whether the client should show a confirmation question on potential dangerous actions.
+    #[serde(default = "Default::default")]
+    pub edit_mode: EditMode,
     /// Whether aliases specified in `pueue_aliases.yml` should be expanded in the `pueue status`
     /// or shown in their short form.
     #[serde(default = "Default::default")]
@@ -173,6 +186,7 @@ impl Default for Client {
             read_local_logs: true,
             show_confirmation_questions: false,
             show_expanded_aliases: false,
+            edit_mode: Default::default(),
             dark_mode: false,
             max_status_lines: None,
             status_time_format: default_status_time_format(),


Test patch:
diff --git a/pueue/tests/client/integration/edit.rs b/pueue/tests/client/integration/edit.rs
index 875da42f..c0fd989d 100644
--- a/pueue/tests/client/integration/edit.rs
+++ b/pueue/tests/client/integration/edit.rs
@@ -1,13 +1,13 @@
 use std::collections::HashMap;
 
 use anyhow::{Context, Result};
-use pueue_lib::task::TaskStatus;
+use pueue_lib::{settings::EditMode, task::TaskStatus};
 
 use crate::client::helper::*;
 
 /// Test that editing a task without any flags only updates the command.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn edit_task_default() -> Result<()> {
+async fn edit_task_directory() -> Result<()> {
     let daemon = daemon().await?;
     let shared = &daemon.settings.shared;
 
@@ -159,3 +159,41 @@ async fn fail_to_edit_task() -> Result<()> {
 
     Ok(())
 }
+
+/// Test that editing a task without any flags only updates the command.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn edit_task_toml() -> Result<()> {
+    // Overwrite the edit mode to toml.
+    let (mut settings, tempdir) = daemon_base_setup()?;
+    settings.client.edit_mode = EditMode::Toml;
+    settings.save(&Some(tempdir.path().join("pueue.yml")))?;
+    let daemon = daemon_with_settings(settings, tempdir).await?;
+    let shared = &daemon.settings.shared;
+
+    // Create a stashed message which we'll edit later on.
+    let mut message = create_add_message(shared, "this is a test");
+    message.stashed = true;
+    send_message(shared, message)
+        .await
+        .context("Failed to to add stashed task.")?;
+
+    // Update the task's command by piping a string to the temporary file.
+    let mut envs = HashMap::new();
+    envs.insert(
+        "EDITOR",
+        "echo '[0]\nid = 0\ncommand = \"expected command string\"\npath = \"/tmp\"\npriority = 0' > ${PUEUE_EDIT_PATH} ||",
+    );
+    run_client_command_with_env(shared, &["edit", "0"], envs)?;
+
+    // Make sure that both the command has been updated.
+    let state = get_state(shared).await?;
+    let task = state.tasks.get(&0).unwrap();
+    assert_eq!(task.command, "expected command string");
+    assert_eq!(task.path.to_string_lossy(), "/tmp");
+
+    // All other properties should be unchanged.
+    assert_eq!(task.label, None);
+    assert_eq!(task.priority, 0);
+
+    Ok(())
+}
diff --git a/pueue/tests/helper/fixtures.rs b/pueue/tests/helper/fixtures.rs
index e9005773..2c53f12b 100644
--- a/pueue/tests/helper/fixtures.rs
+++ b/pueue/tests/helper/fixtures.rs
@@ -168,6 +168,7 @@ pub fn daemon_base_setup() -> Result<(Settings, TempDir)> {
     let client = Client {
         max_status_lines: Some(15),
         status_datetime_format: "%Y-%m-%d %H:%M:%S".into(),
+        edit_mode: EditMode::Files,
         ..Default::default()
     };