Fix regarding files not being skipped, even tho the checkbox is ticked.

[toczekmj]

This pr should resolve #275 I have discovered that in some particular cases a file is saved as a tagged filename instead of Clean name, but the metadata title is still clean. So i think that checking the meta title will help to resolve that issue.

[toczekmj]

I found an error in this PR, please let me resoleve that.

[toczekmj]

Okay, now I think that I've fixed the problem.

Instead of checking only the clean title, now we also cares about tagged title, since there are cases when files are saved in either form.

[shaked6540]

Looks like there are conflicts because I merged the other PR, I'll also need to check the code in more detail later. What I wanted to check orignally, is the name of the final file, if it exists then we add a number to the name (like windows do automatically) but if the "skip exisiting files" is checked its supposed to ignore that

[toczekmj]

That's indeed what should happens now. There are two places in the code, where we checks presence of identical files, and we can get to the 2nd one only if the both "skip if exists" and file presence are false. Thus when we decide not to skip identical files it will proceed to enumerate.

[cristianst85]

@toczekmj, please rebase this PR on the existing master branch and then do a force push.

[toczekmj]

Will do as soon as I get back home, probably within next 24 hours.

[toczekmj]

@cristianst85 should be all set now.

[cristianst85]

Thanks! I know I am being picky, but it's best to squash everything into a single commit (and force push again). This will help keep a clean git history and easier to review. Also, the last merge was a bit too messy (for me anyway).

[toczekmj]

Seems like at this point I cannot squash all those commits, at least Rider does not lets me do this. Bcs if I'm not wrong, what you are trying to accomplish is to squash all commits since 1634d3e6e952eab91e4aee0e16fb6c3cc09fe620 to d72b15f3da1eb059d15cc44b262428dcf6b24ea3, but what I find hard to understand is that there were some updates in main repo, and I did rebase my branch, so it is not linear graph, therefore I quite don't know how can i squash it all together.

[cristianst85]

Try these steps.

1) Sync your (forked) repository with this one. From GitHub. 2) Using your git client (Rider) checkout the master branch and then do a pull with rebase (usually pull with merge will do, but just to be safe). 3) Then checkout your feature/fix branch, and do a rebase on your master branch again. After this you should only have two commits that needs to be squashed. You can use interactive rebase git rebase -i HEAD~2. Mark the second/last commit that you want to squash with s (s is from squash). This commit will be squashed into the previous one. 4) Force push your feature/fix branch to GitHub.

P.S. From what I can see your branch does not branch from the master branch meaning it was not rebased properly. See the blue line it must branch from the latest commit of the master branch.

[toczekmj]

@cristianst85 thank you so much for this. Please take a look if everything is adjusted to your liking now :)

[shaked6540]

Hey guys, you dont have to worry about squashing commits, it can be done automatically when I press on merge

[cristianst85]

@toczekmj, that's perfect!

@shaked6540, it's a learning opportunity for @toczekmj.

[cristianst85]

Still doesn't work properly when File name pattern is set to $index $title because the taggedPath does not contain the index and both File.Exists return false.

Test link: https://www.youtube.com/watch?v=uFLoZ-gr4PE&list=OLAK5uy_nLwrcZJLKqwkwW1k3GLuic1hA9NUchja8

Test settings:

[cristianst85]

Few other notes. I find the code quite difficult to follow. I don't like that TagMusicFile, TagFileBasedOnTitle are called from within the FileExists method. A more elegant way to fix this is to determine the final file name (and path), if possible (!), before the download actually starts and use that name. Unfortunately the code for determining the file name is quite fragmented and this will require a lot of refactoring.

Also, FileExists is not an extension method and the ExtensionMethods class is more like a helper or utility class (should be refactored/renamed, probably in another PR).