$CMD_DURATION Weird format

[giodamelio]

I am trying to make my prompt notify me when a command that has taken a certain amount of time is done. $CMD_DURATION is awesome for that, except its has a really weird output format, which makes it hard to any math with. This does not appear to be used anywhere else, so why the weird output? Wouldn't it be better to just output it in milliseconds?

[ridiculousfish]

See #1279 too, which suggests outputting using the real/user/sys triplet.

[giodamelio]

That would be great, is there any change of that happening any time soon?

[ridiculousfish]

Not as such, but go for it if you feel up to it.

[giodamelio]

Awesome, my C is a little rusty, but this should be simple enough. Just to be clear, you're fine with me getting rid of all that formatting and just returning the duration in ms?

[ridiculousfish]

I don't think you'll find many people attached to the current behavior. Start by proposing any reasonable alternatives you can think of, then give your preference and justify it - pros and cons. Focusing on the use cases is good justification. The use case that prompted your interest (doing math) is a good one, and also think about formatting for display. For example, if the user wanted to show the duration of the last command in the prompt, what would the script to do that look like?

Hope that doesn't scare you off. It's not mean to be some big process, just to put a little thought into the design.

[giodamelio]

Not a problem. I definitely think setting $CMD_DURATION to the number of milliseconds is the way to go.

Pros

• Consistent, in Unix systems, time is just a bunch of milliseconds
• Easy on the math, keeps everything in integers so it is nicely compatible with fish's test command

Cons

• Not backwards compatible. This should not really be a problem since this is an undocumented and largely unused feature.

Example use. Sends user notification if last command took more then ten seconds to execute.

# If a command has run for more then ten seconds, notify the user when it completes
# Put this somewhere in fish_prompt
if test $CMD_DURATION
    if test $CMD_DURATION -gt (math "1000 * 10")
        set secs (math "$CMD_DURATION / 1000")
        notify-send "$history[1]" "Returned $status, took $secs seconds"
    end
end

[arthurfabre]

I second the move to using milliseconds only.
My use case is to compare script execution time, and parsing the $CMD_DURATION format is a bit verbose - I've ended up with the following:

# Check if var is set
if set -q CMD_DURATION

    # Parse the duration into an array - and get rid of decimal points and trailing / leading newlines
    set -l time (echo $CMD_DURATION | sed -r 's/\ *([0-9]*)(\.[0-9]*)?[a-z]*/\1\n/g' | tr -d '\n')

    # If the array has more than two entries (ie command took more than 60 seconds), or the only entry (seconds) is > 5
    if begin; test (count $time) -gt 1; or test $time[1] -gt 5; end
        # Command took more than 5 seconds
    end
end

By comparison, converting a value in ms to "formated" time is a lot simpler - it can be accomplished with the following:

date -u -d @$CMD_DURATION "+%kh %Mm %Ss"

This also allows users to choose their own format.

[giodamelio]

I am having a bit of trouble implementing this(I don't know much c++). When I change out set_env_cmd_duration with this:

void set_env_cmd_duration(struct timeval *after, struct timeval *before)
{
    time_t secs = after->tv_sec - before->tv_sec;
    suseconds_t usecs = after->tv_usec - before->tv_usec;
    wchar_t buf[16];

    if (after->tv_usec < before->tv_usec)
    {
        usecs += 1000000;
        secs -= 1;
    }

    swprintf(buf, sizeof(buf), L"%d", usecs);
    env_set(ENV_CMD_DURATION, buf, ENV_EXPORT);
}

I expect it to set $CMD_DURATION to the number of microseconds that is took the command to run(see man 2 gettimeofday). It is not. If I run sleep 2, it will output number anywhere from about 2500-6500. I have to clue what is going on.

[femnad]

usecs are in microseconds, namely 1 millionth of a second. Also you are ignoring the seconds part of the time difference. Lastly, the size information you provide to swprintf is not correct. You should provide the whole size of the memory to allocate, which would be equal to the size of 16 chars in your case.

[giodamelio]

Ah, it both added together, thanks.

[giodamelio]

It still doesn't make sense to me. If I swap in

swprintf(buf, 16, L"%d|%d", (secs / 1000), (usecs * 1000));

and run sleep 2, it will output something like 0|5137000, that is way too many milliseconds.

[femnad]

I think you should multiply secs and divide usecs if you want the result in milliseconds. But the format string you've selected would produce something like '2000|5' in that case, if you ran sleep 2. Is that what you'd like?

[giodamelio]

Ugg, I can't believe I reversed those. Also, the formatting was just for debugging. Pull request incoming.