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

Fixes for recently-introduced readline and $&time bugs #153

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

jpco
Copy link
Collaborator

@jpco jpco commented Dec 8, 2024

Fixes #151 (introduced with #128) and #152 (introduced with #143).

getrusage() has been pulled out of proc.c entirely and is now managed within $&time. I think this structure is a lot better in terms of separation-of-concerns. It's also nicely simple to implement with how getrusage() works, which is something that seemed very weird to me at first. So that's nice!

It occurs to me with this change that if we make use of getrusage(RUSAGE_SELF) as well, we could actually $&time commands without forking, which seems like an interesting possibility. If you have a slow-running script, you could just wrap whole sections inside a time {} without any disruption to the script's function... Even better, it looks like the standard times() also supports getting times from the calling process, so we could keep things consistent. (Edit: This is now #154)

We don't do anything so clever with rl_reset_screen_size(), even though it's easy to imagine something that waits for a SIGWINCH and only calls resets the screen size then. However, that requires a novel kind of special signal handling for SIGWINCH, and SIGWINCH isn't even technically POSIX.1-2001 standard, so we'd have to wrap a fair amount inside of #ifdef SIGWINCH, and this is all adding a fair amount of complexity for a very negligible performance/efficiency gain.

@jpco jpco changed the title Bugfixes Fixes for recently-introduced readline and $&time bugs Dec 8, 2024
@jpco jpco added the bug label Dec 10, 2024
@jpco jpco merged commit 4bfee21 into wryun:master Dec 13, 2024
1 check passed
@jpco jpco deleted the bugfixes branch December 13, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$&time reports bad numbers in certain situations
1 participant