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: error when vault's rel_dir is a parent directory #32

Merged
merged 6 commits into from
Aug 29, 2024

Conversation

callumtilbury
Copy link
Contributor

@callumtilbury callumtilbury commented Jul 22, 2024

Issue raised here, with a very simple fix actually—just normalise the path before processing.

We could look at having an abs_path argument too, but I don't want to change the API or anything right now. This is an easy change, that simply allows the parent directory (..) to be part of the rel_dir.

@callumtilbury
Copy link
Contributor Author

@jcformanek maybe you could take a quick look at this, to approve? Simple test is to have your vault in the parent directory (..)—by using the old flashbax version, an error will be thrown, but this change should fix 💯

@callumtilbury callumtilbury self-assigned this Jul 22, 2024
@callumtilbury callumtilbury added the bug Something isn't working label Jul 23, 2024
@EdanToledo
Copy link
Contributor

Why are we type ignoring?

@callumtilbury
Copy link
Contributor Author

@EdanToledo apologies, solved this elsewhere with #35. Need to update here :)

@callumtilbury
Copy link
Contributor Author

@EdanToledo fixed. Just waiting to confirm that mypy is now happy 🚀

@callumtilbury
Copy link
Contributor Author

@EdanToledo when you're ready: 🤾🏻‍♂️

Copy link
Contributor

@EdanToledo EdanToledo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gg

@EdanToledo EdanToledo merged commit 0dffe1b into main Aug 29, 2024
3 checks passed
@EdanToledo EdanToledo deleted the fix/support_parent_rel_paths branch August 29, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants