8000 Saturate exit codes to 255 for all builtins by ethulhu · Pull Request #7702 · fish-shell/fish-shell · GitHub
[go: up one dir, main page]

Skip to content

Saturate exit codes to 255 for all builtins#7702

Closed
ethulhu wants to merge 1 commit intofish-shell:masterfrom
ethulhu:saturate-all-builtins
Closed

Saturate exit codes to 255 for all builtins#7702
ethulhu wants to merge 1 commit intofish-shell:masterfrom
ethulhu:saturate-all-builtins

Conversation

@ethulhu
Copy link
@ethulhu ethulhu commented Feb 9, 2021

Description

After commit 6dd6a57, 3 remaining builtins were still affected by uint8_t overflow: exit, return, and functions --query.

This PR:

  • Moves the overflow check from builtin_set_query to builtin_run.
  • Removes a conflicting intuint8_t conversion in builtin_return.
  • Adds tests for the 3 remaining affected builtins.
  • Changes documentation for exit and return.
  • Does not change documentation for functions --query, because it does not state the exit code in its API.
  • Duplicates CHANGELOG lines for each affected builtin; this could equally be condensed into a single changelog entry?

This PR is a generic solution to overflowing exit codes, following discussion on PR #7698.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@ethulhu ethulhu force-pushed the saturate-all-builtins branch from 51e4490 to 69c9f76 Compare February 9, 2021 23:20
@ethulhu ethulhu force-pushed the saturate-all-builtins branch from 69c9f76 to 8e13c83 Compare February 10, 2021 11:23
After commit 6dd6a57, 3 remaining
builtins were affected by uint8_t overflow: `exit`, `return`, and
`functions --query`.

This commit:
- Moves the overflow check from `builtin_set_query` to `builtin_run`.
- Removes a conflicting int -> uint8_t conversion in `builtin_return`.
- Adds tests for the 3 remaining affected builtins.
- Simplifies the wording for the documentation for `set --query`.
- Does not change documentation for `functions --query`, because it does
  not state the exit code in its API.
- Updates the CHANGELOG to reflect the change to all builtins.
@ethulhu ethulhu force-pushed the saturate-all-builtins branch from 8e13c83 to 1a8d282 Compare February 10, 2021 11:25
@krobelus krobelus added this to the fish 3.2.0 milestone Feb 13, 2021
@krobelus
Copy link
Contributor

Great job, thanks! - merged as 5a0aa78

@krobelus krobelus closed this Feb 13, 2021
xdelaruelle added a commit to xdelaruelle/modules that referenced this pull request Apr 5, 2021
Update fish install tests following change introduced on fish 3.2, where
the exit code now saturates to 255 for all builtins commands rather
overflowing. (see fish-shell/fish-shell#7702)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0