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

fix compressed count regression #33

Merged

Conversation

pancelor
Copy link
Contributor

repro: echo 'abc=3' | shrinko8 --count --minify - -

before this commit, this command showed stats for
tokens and chars, but not compressed

now, it also shows compressed


sometime between 15304da and now, the code changed:

# 15304da
if not (args.output and str(args.format) not in CartFormat.src_names) and not args.no_count_compress: # else, will be done in write_cart

# before this commit
if not (args.output and args.format.is_src()) and not args.no_count_compress: # else, will be done in write_cart

The difference is str(args.format) not in CartFormat.src_names -> args.format.is_src(), which is missing a not. fixed!

repro: `echo 'abc=3' | shrinko8 --count --minify - -`

before this commit, this command showed stats for
tokens and chars, but not compressed

now, it also shows compressed
@pancelor
Copy link
Contributor Author

On second though, the # else, will be done in write_cart comment confuses me; it seems out-of-date, possibly? this is the only place in the whole project where write_compressed_size is used

@thisismypassport thisismypassport merged commit de6a137 into thisismypassport:main Oct 17, 2023
1 check passed
@thisismypassport thisismypassport temporarily deployed to github-pages October 17, 2023 03:46 — with GitHub Actions Inactive
@thisismypassport
Copy link
Owner

Oops! - thanks, taken

(write_compressed_size is done to compute & print the compressed size when we won't do this anyway as part of write_cart)

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.

None yet

2 participants