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 editing time entries #86

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Allow editing time entries #86

wants to merge 13 commits into from

Conversation

shantanuraj
Copy link
Member

@shantanuraj shantanuraj commented Dec 11, 2024

What does this PR do?

This adds support for editing time-entries.

$ toggl edit # To edit the currently running time-entry.
$ toggl edit -i # To open an interactive picker to select a time-entry to edit.

This opens the time-entry as a text file in the default editor.1
The user can then edit the time-entry and save the file.
The time-entry is then updated with the new values.

The core of this lies in the Parcel trait, which allows us to serialize and
deserialize the time-entry into a sequence of bytes.

pub trait Parcel {
    fn serialize(&self) -> Vec<u8>;
    fn deserialize(&self, data: Vec<u8>) -> Self;

    fn launch_in_editor(&self) -> Result<Self, String>
    where
        Self: Sized,
    {
        let contents = self.serialize();

        let dir = tempdir().expect("Failed to create temp directory");
        let file_path = dir.path().join("toggl.txt");

        let mut file = File::create_new(file_path.clone()).expect("Failed to create file");
        file.write_all(&contents)
            .expect("Failed to write current time-entry to file");

        utilities::open_path_in_editor(&file_path).expect("Failed to open file in editor");
        drop(file);

        let contents =
            fs::read(file_path).expect("Failed to read file time-entry editing in editor");

        dir.close().expect("Failed to clear temp directory");

        Ok(self.deserialize(contents))
    }
}

I'm still ironing out the details of this, but it seems to be working well so
far. And the launch_in_editor method should probably be moved to a separate
trait, as it's not really related to serialization/deserialization.

Changes

  • 🎬 Allow editing time-entries
  • ✍️ Use File::write_all to write time-entry
  • ⛷️ Serialize directly into bytes instead of json
  • 🌳 Implement project/task deserialization

Todos

  • Allow updating time-entry using editor
  • Allow editing running time-entry
  • Allow editing last time-entry
  • Allow editing specific time-entry
  • Handle billable correctly
  • Handle a line being removed correctly, ie. unset the field
  • Implement interactive picker for selecting time-entry to edit
  • Accept time-entry edits from flags
  • Implement nicer fallback for editor
  • Maybe add editor configuration to toggl config

Gif

editing

Context

TODO


Footnotes

  1. The default editor is determined by the EDITOR environment variable.
    If this is not set, we default to vim. It might be worth to add smarter
    fallbacks, like checking for VISUAL or git config core.editor.
    In addition to adding our own variable TOGGL_EDITOR in case the user
    wants to use their editor with different configuration for toggl.

@shantanuraj shantanuraj added the enhancement New feature or request label Dec 11, 2024
@shantanuraj shantanuraj self-assigned this Dec 11, 2024
@wd60622
Copy link

wd60622 commented Dec 11, 2024

I haven't done any programming in rust (yet) but I am building from source and will give it a try!

@shantanuraj
Copy link
Member Author

That's great! If you run into any issues feel to reach out.

@wd60622
Copy link

wd60622 commented Dec 11, 2024

Would it make sense to also support flags here for non-interactive mode. For instance, toggl edit --description "new description" would only change the description.

Only toggl edit --interactive would open the editor in my mind. What do you think?

@shantanuraj
Copy link
Member Author

That makes sense, it is quite similar to start in principle.

The only limitation it's not trivial to choose which time-entry to edit.
For now this PR only supports editing the running entry.
We could default to last-entry if there's no running entry.

But yeah for this reason I was hoping to use the --interactive flag to select
the time-entry you'd like to edit.

Another option could be add update in addition to the edit command
which operates entirely using cli arguments, whereas edit does the same thing
but allows you to update the entry as text. Somewhat similar to the toggl config edit command.

@wd60622
Copy link

wd60622 commented Dec 11, 2024

Ah, I see. I had the impression that it was for the current running entry only

@shantanuraj
Copy link
Member Author

Yeah at the moment that's the case, and I feel 90% of the time that is how it will end
up being used.

I feel it'd be nice if edit works for for any entry so you could use the cli to list entries
and you if see something out of place you can correct it from within the cli.

Though if you feel strongly in either direction let us know!
I'm quite open to changing how this works.

@shantanuraj
Copy link
Member Author

@wd60622 I've added a Todos list to the PR description, and pushed some changes so editing the running entry now works with the editor of your choice and toggl edit.
You can update description, tags, project, task and start time for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants