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

Implementation of --maxdepth to limit recursion of input directory #104

Merged
merged 11 commits into from
Mar 30, 2021

Conversation

unapproachable
Copy link
Contributor

Implementation of --maxdepth from #92.

No tests were written because it's actually beyond my capabilities to write the test. Maybe someone can help.

Merge collisions exist from the prior commit, possibly related to the Windows platform, but I'm unclear how to resolve them.

unapproachable and others added 5 commits March 16, 2021 12:41
Arbitrary limit of 255 set for command line option
ivandokov#92
* Migrated to a more compatible filename for testing exiftool

* --Migrated Windows escaping to quoted input rather than shlex.quote escaping.
--Added space and punctuation test
--Added additional test image

* --Updated trailing slash test to use Windows specific slashes on win32

* --Updated trailing slash test to use Windows specific slashes on win32

* --revert unintended commit
# Conflicts:
#	phockup.py
#	src/phockup.py
@ivandokov
Copy link
Owner

You have to pull master and go to your branch feature/maxdepth and do git merge master to merge the latest changes to your feature branch. Then you will have to resolve te conflicts locally and when you do you can push the update on your branch and then we will see a cleaner diff in this PR.

@unapproachable
Copy link
Contributor Author

Thanks for the suggestion. I am pretty sure I did this:

P:\Projects\phockup>git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.

P:\Projects\phockup>git pull
Already up to date.

P:\Projects\phockup>git checkout feature/maxdepth
Switched to branch 'feature/maxdepth'
Your branch is up to date with 'origin/feature/maxdepth'.

P:\Projects\phockup>git merge master
Already up to date.

P:\Projects\phockup>

Looking at the diffs, I can't tell what's different between my feature branch and your master. I'll maybe try updating from upstream again and see if I can get it to sync up. I was current with the quiet mode PR on my fork, but I must have borked something. I hope it isn't that weird filename causing issues pulling/pushing to Windows.

@ivandokov
Copy link
Owner

Appologies for the misleading steps. Here is a proper guide how to sync your repo with the upstream https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/syncing-a-fork

@unapproachable
Copy link
Contributor Author

Okay... so, I moved over to Linux for the upstream work to get the changes pulled (same steps as on Windows), but here they work. I'm pretty sure the weird filename was preventing something from actually going through, but it was failing silently. Every time I tried to fetch upstream and merge to master or my branch I had the same error with the file and regardless of how I dealt with it, it came back if I re-ran the fetch.

Anyway, we look clean now for merging. I'm just working through the issues list to get practice in Python and since I use phockup. Hopefully some value added here.

@ivandokov
Copy link
Owner

Looking good now. Can you please add information about the new flag in the readme.

unapproachable and others added 2 commits March 26, 2021 18:17
Updated defaults to support --maxdepth=0 to allow processsing only the input directory without any subdirectory traversal.
@ivandokov ivandokov merged commit edde954 into ivandokov:master Mar 30, 2021
@unapproachable unapproachable deleted the feature/maxdepth branch September 28, 2024 13:02
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.

3 participants