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

refactor : --rm ignored with stdout #3443

Merged
merged 2 commits into from
Jan 23, 2023
Merged

refactor : --rm ignored with stdout #3443

merged 2 commits into from
Jan 23, 2023

Conversation

Cyan4973
Copy link
Contributor

@Cyan4973 Cyan4973 commented Jan 21, 2023

zstd CLI has progressively evolved towards a policy ignoring --rm command when the output is stdout. The primary drive is to have a behavior consistent with gzip where --rm is the default, but --rm is ignored when output is stdout. Other policies are certainly possible, but would break from this gzip convention.

The new policy was inconsistenly enforced, depending on the exact list of commands. For example, it was possible to circumvent it by using -c --rm in this order, which would re-establish source removal.

  • Update the CLI so that it necessarily catches these situations and ensures that --rm is always disabled when output is stdout.
  • Added a warning message in this case (for verbosity 3 -v).
  • Added an assert(), which controls that --rm is no longer active with stdout
  • Added tests, which control the behavior (including even when --rm is added after -c)
  • Removed some legacy code which where trying to apply a specific policy for the stdout + --rm case, which is no longer possible

`zstd` CLI has progressively moved to the policy of
ignoring `--rm` command when the output is `stdout`.
The primary drive is to feature a behavior more consistent with `gzip`,
when `--rm` is the default, but is also ignored when output is `stdout`.
Other policies are certainly possible, but would break from this `gzip` convention.

The new policy was inconsistenly enforced, depending on the exact list of commands.
For example, it was possible to circumvent it by using `-c --rm` in this order,
which would re-establish source removal.

- Update the CLI so that it necessarily catch these situations and ensure that `--rm` is always disabled when output is `stdout`.
- Added a warning message in this case (for verbosity 3 `-v`).
- Added an `assert()`, which controls that `--rm` is no longer active with `stdout`
- Added tests, which control the behavior, even when `--rm` is added after `-c`
- Removed some legacy code which where trying to apply a specific policy for the `stdout` + `--rm` case, which is no longer possible
@Cyan4973 Cyan4973 merged commit ced0882 into dev Jan 23, 2023
@Cyan4973 Cyan4973 deleted the no_rm_w_stdout branch January 26, 2023 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants