VladimirMarkelov / ttdl

TTDL - Terminal Todo List Manager
MIT License
210 stars 17 forks source link

Feature Request: New Optional Key-Value Param. For Estimated Task Duration #64

Closed MikeTheSapien closed 1 year ago

MikeTheSapien commented 1 year ago

Hey there, first, great thanks to your work.

Would it be possible to add a key-value parameter to associate an estimated duration for a particular task?

It is my curiosity to perhaps see how small or huge the variance might be of the estimated time it would take to finish the task compared to the actual time it took.

In my limited understanding of your code, it seems each task is represented by the type Task in your project todo_lib. May I know, if you have, your warnings if I would just "simply" add a field to that type? Perhaps Task { .., est_dur: Option<std::time::Duration> }? After adding, should I just rely on compiler errors to update parts of your code that would be affected? Am I approaching this stupidly?

Thanks for reading.

VladimirMarkelov commented 1 year ago

Thank you!

When I developed TTDL, I tried to keep todo_lib minimal and put to it only features that are in todo.txt description or hard to implement outside of the library(e.g, it would have made sense to implement done and undone in the library even if it were not described in todo.txt). That is why all extensions, including time tracking ones, are implemented in the binary. Hence, in my opinion, it is better to add the new tag and its processing to the binary.

So, I would do it this way:

  1. Choose a tag name
  2. Add a new column and hide it by default
  3. If the column is visible just print the tag value. I am not sure if it can be in short and long format but it is possible to squeeze duration to one item in short mode(e.g, show 2h instead of 1h45m etc; maybe round up always)

Extra points of interest:

  1. It needs a utility function to convert human-readable duration to a chrono duration. I do not think it is good to make people to set duration in seconds or alike, nobody will do it. It is good to add tests for the function: 40 = 40 seconds, 89m = 1 hour and 29 minutes, 1h70s = 1 hour 1 minute 10 seconds etc. Intervals greater than a week are not supported because neither months nor year has a constant length.
  2. I think, it would be good to add a generic method to a todo in todo_lib - it may be useful in the future - something like update tag value. Really, there is already such function edit, we need only to extend Config with a new fields to update an arbitrary tag or list of tags. This step may trigger some refactoring because the existing calls to update due, threshold etc may be simplified a bit.
  3. In theory, it is also good to have a way to edit duration. We can add a generic command-line option --set-tag tag-name:tag-value. And maybe support --tag:[value] for list command. It also can trigger updating todo_lib to extend filter config.

As you see, a simple feature - at first sight - turns into a big bunch of tasks. But it must be fun :) I think I'll start with adding generic methods to todo_lib. It will result in changing major version of the library version as Configs are going to change.

MikeTheSapien commented 1 year ago

I'm sorry my response is taking up too much time, relative to your quick response.

I'll still compose a proper response to your suggestions and insights.

And a more important note, I would love to help you with your planned changes! To me, this tool of yours is amazing! Actually I liked this because it just so happens that I frequently use and create chat bots so I could "take my app everywhere" without a laptop.

It's also quite obvious that you have more experience than I and so I'll just listen to your suggestions and would ask if something seems off or doesn't make sense.

edit: PS. I love regex, I use it a lot in my codes. Even if there are tools like getopts, I created my own command line input parser, for me to improve my knowledge as well on code design and structures.

VladimirMarkelov commented 1 year ago

Making your own tools to learn is a good way to improve skills and to understand how it works under the hood ;-)

In my case, I choose a crate that had minimal imprint. Moreover, I used getopt before in other languages, so I was kind of familiar with it. If I started TTDL today I might choose another crate, I think.

As of changes, I have already added a function to update any custom tag in a todo. It is in a separate branch. I want to add a feature I spotted in another library: hashtags. Maybe, it would be useful for someone. When I finish with hashtags (basic support: add/remove/filter), I'll publish the new todo_lib 6.0,0. After that the new features should be used by TTDL: add new options for tags and hashtags to list and edit commands. Everything, I mentioned above, is also should be implemented.

You can plan what you want to get from the new "property". First, we need a tag name. e seems to short, est looks better. Maybe you have another good ideas. A name for column in the output. Then, even with the current todo_lib, without waiting for 6.0.0, you can try adding a new column in the output where you print your tag value (now all "unknown" tags are in the HashMap tags). At this moment, I do not think we need a conversion between 4h and duration - because we do not use the real value of the estimation in any way, we only print it.

Adding options to modify and filter, I think, can be the last step.

MikeTheSapien commented 1 year ago

(e.g, it would have made sense to implement done and undone in the library even if it were not described in todo.txt). That is why all extensions, including time tracking ones, are implemented in the binary. Hence, in my opinion, it is better to add the new tag and its processing to the binary.

Wow this is smart


  1. Add a new column and hide it by default

What do you mean by this? Do you mean the array fmt::FIELDS?


it needs a utility function to convert human-readable duration to a chrono duration. I do not think it is good to make people to set duration in seconds or alike, nobody will do it. It is good to add tests for the function: 40 = 40 seconds, 89m = 1 hour and 29 minutes, 1h70s = 1 hour 1 minute 10 seconds etc.

I agree with the setting of seconds being unnecessary.

Do you think the function signature should be: read_chrono_dur(chrono::Duration) -> String to convert from Duration into human-readable? Or do you think I should create some type to represent duration in hours, minutes, and seconds that would then implement fmt::Display? so read_chrono_dur(chrono::Duration) -> impl fmt::Display

The inverse of that function would be into_chrono_dur(String) -> Result<chrono::Duration, String>, or it can have a function parameter to take some type representing the duration input that is human-readable and that implements FromStr? So into_chrono_dur<T: FromStr>(T) -> Result<chrono::Duration, String>


I think, it would be good to add a generic method to a todo in todo_lib - it may be useful in the future - something like update tag value. Really, there is already such function edit, we need only to extend Config with a new fields to update an arbitrary tag or list of tags. This step may trigger some refactoring because the existing calls to update due, threshold etc may be simplified a bit.

I think you've made some recent changes to todo_lib, is your statement above related to that?


Do you think this generic method to edit the tags can be used for a possible bug? The possible bug I'm uncertain of is when a recurring task is repeated, and it has a time spent set on to it, the time spent is passed on to the new task created. As a work around, I just edit the .txt file that holds the records and delete the substring that holds the data for time spent.

MikeTheSapien commented 1 year ago

Reading your reply 4 days ago, compared to the one I tried to reply to last week, I think you answered all my questions in the preceding comment I sent. So I guess I'll close this then?

VladimirMarkelov commented 1 year ago

As the feature is not implemented yet(if we want to have a separate column to show the estimated time), we can keep the issue opened.

What do you mean by this? Do you mean the array fmt::FIELDS?

Yes. Start from appending a column to the FIELDS. Then think of expected width of the column, and update calc_width. Then update print_header_line and print_line to process the new column. Then check that --field=<NEW-FIELD> shows the new column correctly. It is enough for the start. Later it may make sense to show different ranges with different color (e.g, > 1 day = red, < 20 minutes = green, the others = default color).

Do you think the function signature should be: read_chrono_dur(chrono::Duration) -> String?

We need this kind of function only if we want to implement some operations on estimations (e.g, summary). At this moment I do not have in my mind what operations can be useful. So, in the beginning, we can just show estimation as-is, no any conversion. The opposite function str_to_duration(sL &str) -> chrono::Duration can be more useful, e.g, if we are going to implement coloring for the column(see above).

I think you've made some recent changes to todo_lib, is your statement above related to that?

Yes, I've added methods and function to todo_lib for this feature. Now you can add any number of custom tags to TTDL but you do not have to update todo_lib every time - the library provides a generic functions to manage tags.

VladimirMarkelov commented 1 year ago

Today I was thinking about this feature and it dawned upon me: what if another person asks for another column, and another, and another. In this case, the number of fields increases a lot but only few people use the new fields. What if TTDL provides a way to define custom columns that show the value of given tags? So, if a user wants to show some tag value in a designated column, the user modifies the application configuration and that is all. It is harder for an end user as it requires some TOML knowledge, but it is very flexible and the user does not have to wait until the new version of TTDL is published.

VladimirMarkelov commented 1 year ago

I managed to add user-defined columns, so soon you'll be able to show not only estimation column, but even any arbitrary tag value. I haven't merged the patch to the master branch. But you can check the branch custom-col and read the section Custom columns in the README.md(there is a small screenshot by the end of the section). I keep it in the branch for a few days in case of you have any extra ideas or suggestions about the implementation.