Emit only sane pgid value for jobs output#10833
Merged
mqudsi merged 1 commit intofish-shell:masterfrom Nov 8, 2024
Merged
Conversation
We were previously printing the internal INVALID_PID value (since removed), which was a meaningless `-2` constant, when there was no pgid associated with a job. This PR changes that to `-` to indicate no pgid available, which I prefer over something like `0` or `-1`, but will cause problems for code that is hardcoded to convert this field to an integral value. Other options include 0 or -1 as mentioned above, or printing fish's own pgid.
Contributor
|
LGTM. This fixes the output of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We were previously printing the internal INVALID_PID value (since removed), which was a meaningless
-2constant, when there was no pgid associated with a job.This PR changes that to
-to indicate no pgid available, which I prefer over something like0or-1, but will cause problems for any code out there that is hardcoded to convert this field to an integral value (do we have any code like that ourselves?).Other options include 0 or -1 as mentioned above, or printing fish's own pgid.
EDIT:
I don't recommend printing 0 or fish's own pgid because if those get through validation we could end up killing ourself or something equally disasterous.