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

bug: Use en_US when fetching EPOCHREALTIME #2172

Merged
merged 1 commit into from
Dec 3, 2022

Conversation

davidpfarrell
Copy link
Contributor

Isolates fetching of EPOCHREALTIME to a function which sets LC_ALL=en_US.UTF-8.
This ensures that the value is in decimal format, regardless of runtime locale.

Motivation and Context

fixes #2171

How Has This Been Tested?

Tested locally

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@davidpfarrell
Copy link
Contributor Author

@PinheiroCosta Can you test this branch in your local and report back on how it goes?

@PinheiroCosta
Copy link

@PinheiroCosta Can you test this branch in your local and report back on how it goes?

Thank you @davidpfarrell, it's working fine now. Check it out:

localefix

@davidpfarrell
Copy link
Contributor Author

@PinheiroCosta thanks for confirming!

Quick Q: I don't use duration, can you tell me if this is normal/expected?
duration-keep-pressing-enter

Seems like if I don't run any commands then I shouldn't see any duration report ...
This also breaks the cmd-returns plugin as, once the value gets above 5, it beeps on every return.

What happens in your local if you just keep pressing enter for several seconds?

@PinheiroCosta
Copy link

PinheiroCosta commented Oct 28, 2022

I believe this happens because the command is computing something like (timestamp_the_last_command_started - now).
When we press the enter key several times, the last command is still running. If we alternate pressing enter key several times with some random commands, this is what we get:

weird-behavior

In my opinion, this shouldn't happen cause it may confuse some devs.

Edit: I don't consider it a bug, but it should be improved. Maybe we should open an enhancement issue for that?

@davidpfarrell
Copy link
Contributor Author

@PinheiroCosta Thanks for confirming I wasn't the only one seeing the behavior.

I don't consider it a bug, but it should be improved

So you'll see I just added a commit which prevents duration from showing when you "just press enter" without running a command.

There are a few pieces to the fix, but the key is this line here :

_bash_it_library_finalize_hook+=("safe_append_prompt_command '_command_duration_pre_cmd'")

If you like you can change this in your local (assuming you're on this PR branch) to be this:

_bash_it_library_finalize_hook+=("safe_append_prompt_command '_command_duration_pre_exec'")

With this change, pressing enter will show how long the prompt has been idle. ie. if you wait 5 seconds and press enter, it will show 5s, if you wait 10 seconds to press enter, it will show 10s, etc ...

For me, showing anything when you "just press enter" on a blank prompt is a bug, so I'm happy with this fix as part of the PR.

If you decide you truly need to know how long you're prompt has been idle, feel free to submit an enhancment PR after this merges -- maybe we can support an extra env variable to switch between the two behaviors ...

@PinheiroCosta
Copy link

Hey @davidpfarrell, awesome solution you've elaborated. I usually measure how long my prompt has been idle by activating the time stamp on the prompt. This additional commit has not just solved my problem but also improved the tool I enjoy using. Thank you!!

Isolates fetching of EPOCHREALTIME to a function which sets LC_ALL=en_US.UTF-8.
This ensures that the value is in decimal format, regardless of runtime locale.

bug: Hide duration when no command executed
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix @davidpfarrell !

@NoahGorny NoahGorny merged commit ad2b558 into Bash-it:master Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: command_duration is broken on different locales than en_US.UTF-8
6 participants