search

dominikbraun / timetrace

A simple CLI for tracking your working time.
Apache License 2.0
685 stars 76 forks source link

Command `timetrace edit record <key>` does not rename file #89

Open joshuaherrera opened 3 years ago

joshuaherrera commented 3 years ago

Similar to issue #81 , the timetrace edit record <key> command does not rename the file. This shouldn't be an issue when we pass either the --plus or --minus flags, since these edit the end times, but without those flags, the user can edit the json file and potentially change the start time, which is the key for a record. The documentation recommends using timetrace list records in conjunction with the timetrace edit record command, but this does not work as intended if a flag is not used.

The code expects the key to correspond to the filename, but since it is not changed, the file is not found.

In the image, I demonstrate the above with record 1. Note that this also occurs when use12Hours is false. editrecord

I'd imagine the solution would be similar to those mentioned in issue #81 .

dominikbraun commented 3 years ago

Thanks for opening this issue! Since the issue that proposed the edit record command stated that only the end time should be changed with --plus and --minus and the behaviour of edit without flags hasn't been specified, I labeled this as an enhancement and not a bug.

You're right that it is the same problem as with #81. I think we should take the same approach for both issues.

dominikbraun commented 3 years ago

So this is my suggestion to solve this:

  1. When edit record is run without options for directly editing the record, load the record internally first.
  2. Store the record's start time.
  3. When the user has finished editing and the start time has changed, rename the file accordingly.

It probably would also make sense to warn the user before the editor opens:

Warning: Directly editing the record can lead to undefined behavior.

If you change the start time of the record, the underlying file will be renamed automatically. Make sure it doesn't collide with other records. If you want to change the end time, use --minus or --plus instead.

Do you want to continue [y/N]?
joshuaherrera commented 3 years ago

@dominikbraun Very well though out solution to this, I like it :+1:

joshuaherrera commented 3 years ago

@dominikbraun I can handle this issue, can you assign it to me please?