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

store: use fixed length hex hashes for dir names #239

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

N-R-K
Copy link
Contributor

@N-R-K N-R-K commented Nov 15, 2024

mostly cosmetics. having fixed width dirnames results in a nicer listing of the directory. it also matches usual expectation of hashes being stringified in hex format.

image

mostly cosmetics. having fixed width dirnames results in a nicer
listing of the directory. it also matches usual expectation of
hashes being stringified in hex format.
@cdown
Copy link
Owner

cdown commented Nov 15, 2024

I'm not sure I understand the justification, could you go over it more?

@N-R-K
Copy link
Contributor Author

N-R-K commented Nov 16, 2024

With the current naming, directories would look something like this:

image

With fixed-width hex naming, they all have the same width and appear nicer when listing (or when viewed thru a file manager etc).

It's entirely a cosmetic/aesthetic thing. Figured I'd raise it before there's a stable version released in case you like it, otherwise I can just keep the patch locally if you don't.

@cdown
Copy link
Owner

cdown commented Nov 24, 2024

I was on the fence because while it does make hashes more uniform, it also adds (ok, not much) more work and code. I do think it looks better and is also visually more obvious when seeing clip hashes in CM_DEBUG, though, so let's do it. Thanks!

@cdown cdown merged commit 64c335a into cdown:develop Nov 24, 2024
1 check passed
@N-R-K N-R-K deleted the hex-hashes branch November 24, 2024 08:07
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.

2 participants