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

Replace wc by stat in gz #1044

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Replace wc by stat in gz #1044

wants to merge 6 commits into from

Conversation

Rudxain
Copy link

@Rudxain Rudxain commented Mar 16, 2023

fixes #1042 . Only did 1 unit test, I haven't tested the whole fn

@Rudxain Rudxain changed the title replace wc by stat replace wc by stat in gz Mar 16, 2023
@LucasLarson
Copy link

Thanks to your issue (#1042), I just found out that stat -c is GNU, but that macOS/FreeBSD use stat -f. Also, their format flags don’t match (!) but these both return file size:

# GNU
stat -c '%s' file

# macOS/FreeBSD
stat -f '%z' file

But a heads up, you should add the -L argument (same on both platforms) so that even calling a symlink returns the results you want.

@Rudxain
Copy link
Author

Rudxain commented Mar 16, 2023

stat -c is GNU, but that macOS/FreeBSD use stat -f. Also, their format flags don’t match

Thank you for the info! I'll try to make it cross-platform

But a heads up, you should add the -L argument (same on both platforms) so that even calling a symlink returns the results you want.

That's also useful. I forgot about symlinks

@Rudxain
Copy link
Author

Rudxain commented Mar 16, 2023

I'm getting an error

stat -cL '%s' -- .bash_history
# prints this...
# stat: cannot statx '%s': No such file or directory

@LucasLarson what am I doing wrong?

@Rudxain Rudxain changed the title replace wc by stat in gz replace wc by stat in gz, and add sizeof() Mar 16, 2023
@LucasLarson
Copy link

You want this:

stat -Lc '%s' -- .bash_history

because %s is an argument of the -c option.

@Rudxain
Copy link
Author

Rudxain commented Mar 17, 2023

because %s is an argument of the -c option.

I forgot that flag-ordering matters in rare cases.

Fixing now!

@Rudxain
Copy link
Author

Rudxain commented Mar 17, 2023

Wait, I just noticed targz uses both stats, without a uname -s-based condition. I'll use that "hack" (not really a hack, because it's the simplest most reliable way known)

@Rudxain Rudxain changed the title replace wc by stat in gz, and add sizeof() replace wc by stat in gz Mar 17, 2023
@LucasLarson
Copy link

You may want to keep the -L option dropped in 72303304b8

@Rudxain
Copy link
Author

Rudxain commented Mar 17, 2023

You may want to keep the -L option dropped in 72303304b8

Silly me 😅. I forgot the L when copy-pasting

@Rudxain
Copy link
Author

Rudxain commented Mar 17, 2023

Now it's ready (for real, haha)

I've been busy, and thinking of many things yesterday. It seems my cognitive load was higher than I expected. I apologize (to everyone) for the little mistakes

@Rudxain Rudxain changed the title replace wc by stat in gz Replace wc by stat in gz Mar 17, 2023
Okeanos added a commit to Okeanos/dotfiles that referenced this pull request Apr 10, 2023
Okeanos added a commit to Okeanos/dotfiles-windows that referenced this pull request Apr 10, 2023
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.

gz() is inefficient
2 participants