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 #72: mkdir for datafile if necessary #73

Merged
merged 3 commits into from
Jan 27, 2023
Merged

Fix #72: mkdir for datafile if necessary #73

merged 3 commits into from
Jan 27, 2023

Conversation

mattmc3
Copy link
Contributor

@mattmc3 mattmc3 commented Dec 28, 2022

See #72

@mattmc3
Copy link
Contributor Author

mattmc3 commented Dec 28, 2022

Also, I verified that if ZSHZ_DATA is set to a file name with no directory path, the :h suffix will helpfully return '.'. This will ensure that mkdir -p succeeds on that edge case.

ZSHZ_DATA=my_z_database

# the h param expansion returns '.' in this case
echo ${ZSHZ_DATA:h}

# mkdir -p won't fail on this edge case
mkdir -p ${ZSHZ_DATA:h}

@agkozak
Copy link
Owner

agkozak commented Dec 30, 2022

Thank you for the very clever solution.

I wonder if we should do anything further about the edge case. If I accidentally put in my .zshrc

ZSHZ_DATA=quux

instead of

ZSHZ_DATA=~/quux

Zsh-z will happily create a quux file in every directory I navigate to. That is literally what I told it to do, but it's presumably never what I want.

I should be able to address this issue early next week.

@mattmc3
Copy link
Contributor Author

mattmc3 commented Jan 4, 2023

Zsh-z will happily create a quux file in every directory I navigate to. That is literally what I told it to do, but it's presumably never what I want.

That's a great callout. I pushed another one-liner commit that addresses this:

[[ "${datafile:h}" !=  '.' ]] || datafile="${ZDOTDIR:-$HOME}/$datafile"

Let me know if how I handled it works for you. It might seem a bit clever, but I think it reads pretty clear that either the basedir isn't '.' (everywhere), or set the dir to where zsh considers home.

@agkozak agkozak merged commit c28d8f5 into agkozak:master Jan 27, 2023
@agkozak
Copy link
Owner

agkozak commented Jan 27, 2023

Your addition was great. I just merged it to master. Thanks again!

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