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

Display date in pueue status #212

Closed
codekoala opened this issue Jun 23, 2021 · 11 comments
Closed

Display date in pueue status #212

codekoala opened this issue Jun 23, 2021 · 11 comments
Labels
t: Feature A new feature that needs implementation

Comments

@codekoala
Copy link

Is your feature request related to a problem? Please describe.
This is not a problem at all, simply a convenience.

Describe the solution you'd like
I think it would be useful to see the date associated with each task. I have cronjobs that queue tasks, and readily seeing date for each task would be quiet nice.

Describe alternatives you've considered
I have considered using pueue status --json and piping the output through jq to extract date information. Eh.

Additional context
Perhaps the visibility of the date could be configurable?

@codekoala codekoala added the t: Feature A new feature that needs implementation label Jun 23, 2021
@Nukesor
Copy link
Owner

Nukesor commented Jun 23, 2021

I guess one could do this with a few simple rules.

  1. If the start/end is today, just skip the date.
  2. If the start/end is at the same date in the past, only show the date in the start row.
  3. Otherwise show the date in both.

It's just a proposal, I guess we would just have to see at how it looks.

This logic is solely in the pueue client, so we don't have to touch any daemon/shared library logic.

Edit:
We would also have to think about how to format this.
I didn't include the date from the get-go, since it takes a lot of horizontal screen space.
This can probably be mitigated, by adding a newline after the date.

@codekoala
Copy link
Author

I like what you've proposed. As far as date formatting goes, I think it would be safe enough to use ISO-8601 formatting %F or %Y-%m-%d. Perhaps the format could be a configuration option, defaulting to ISO-8601?

@Nukesor
Copy link
Owner

Nukesor commented Jun 23, 2021

Let's just go with %Y-%m-%d, since it's the same format used for the delay:

row.add_cell(Cell::new(enqueue_at.format("%Y-%m-%d\n%H:%M:%S")));

I don't think we need an option to configure this. Unless somebody complains and comes up with a good use-case, there's no need to increase the code's complexity :)

@Nukesor
Copy link
Owner

Nukesor commented Jun 24, 2021

FYI I started working on this.

@Nukesor
Copy link
Owner

Nukesor commented Jun 24, 2021

Hm. I'm not 100% sure which one is the best.
All of them feel kind of cluttered tbh.

No newline:
image

Newline:
image

I think I like this one better than the first one.
The fact that the dates are side-by-side, makes it much easier to compare the two dates.
The additional vertical line is definitely a negative point.

Newline and End only contains the date, if the task didn't finish today:
image

I guess I like the last one the most.
That way you get an instant clue which tasks finished today and it reduces the amount of dates you have to visually parse.

An alternative to this would probably be some relative words like "today", "yesterday", "later that day" etc.

@Nukesor
Copy link
Owner

Nukesor commented Jun 25, 2021

One possible alternative which could be a nice replacement for the End only contains the date, if the task didn't finish today thingy.
We could add some color/hightlight to add the tasks that finished today.

@codekoala
Copy link
Author

I just tried out the changes in #214 and it looks great to me! Thank you very much!

I think I personally would lean toward no newline between the date and time. I've noticed that everything will normally be on a single line if you have a wide terminal, but if you have a skinnier terminal the command and path will wrap across multiple lines (but not the start/end date if the newline is removed from the format string). I think it would be nice if that wrapping behavior worked similarly for the start/end columns: on wide terminals, keep date and time on a single line; on smaller terminals, show date and time on separate lines.

I tried mucking around with https://docs.rs/comfy-table/3.0.0/comfy_table/enum.ColumnConstraint.html#variant.MaxWidth to get that working. I'm not sure if that's the right kind of solution (new to rust, never used comfy-tables etc).

Thoughts?

@Nukesor
Copy link
Owner

Nukesor commented Jun 25, 2021

Yep, the constrains could be a good solution for this. Something like MaxPercent(15).
There are 7-9 Columns.

  • Index always taks ~8 chars.
  • Status can take a little more if a dependency failed (Dependency failed) -> 17 chars
  • Deps takes 4 chars
  • Enqueue at 21 chars
  • Exitcode takes 9 chars
  • command and path are variable and should get as much space as possible
  • start/end take either 8 (time only) to 12 (datetime with newline) or 21 (full datetime).

A full 1080p screen with (at least for me) normal font size has about 240 Columns.
15 Percent would mean that the content is split as soon as you hit a 140 Columns limit.

Worst case (dependency failed + deps + enqueue at) - 8*2 padding
(140-8-17-4-21-9-8*2)/2 = 32 chars for command and path each. That's not much

Best case (no dependency failed, no deps, no enqueue at) - 6*2 padding
(140-8-10-9-6*2)/2 = 54. This is closer to acceptable

I guess we would have to experiment a little and we also might have to adjust the absolute max percentage depending on the amount of columns. MaxPercentage also comes with a caveat, where the date is butchered as soon as the terminal has too few columns (80 columns). At that point, the date doesn't fit into a single column.

An alternative to this would be to use MaxWidth, as you suggested, and set this depending on the current width of the terminal.

@Nukesor
Copy link
Owner

Nukesor commented Jul 5, 2021

@codekoala After some consideration, I actually went ahead and added a config option, which allows users to customize the time and datetime format string.
That way everybody can specify their own style.

I did this, as I'm personally in favor of a newline, but this is obviously personal preference.
Feel free to check out the new state of the feature.

FYI. The branch is based upon some other heavy refactorings, which introduce breaking non-backward compatible changes.
It's best to restart the daemon after updating!

@Nukesor Nukesor closed this as completed Jul 7, 2021
@codekoala
Copy link
Author

@Nukesor that sounds great to me! Thank you! I'll give it a whirl today.

Sorry for the delayed response--I was out of town for a long weekend and have been playing catch-up for the past couple of days.

@codekoala
Copy link
Author

Just tried it out, and it's working wonderfully for me! ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: Feature A new feature that needs implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants