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

Episode path format #108

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Episode path format #108

wants to merge 16 commits into from

Conversation

fracai
Copy link
Contributor

@fracai fracai commented Jun 4, 2021

This should close #70 by replacing the "Append Date" and "Append Episode Number" with additional formatting tokens that can be combined however desired. It also adds support for path separators in the format to introduce additional folder structures. The show title is still used as the base directory.
Something like: %YYYY%/%YYYY%-%mm%/%YYYY%-%mm%-%dd% %EpisodeTitle% would result in a file saved to something like: /assets/This American Life/2021/2021-05/2021-05-28 Good Grief.mp3

<input type="checkbox" name="appendEpisodeNumberToFileName" v-model="appendEpisodeNumberToFileName">
<span class="label-body">Append episode number to episode file name</span>
<label for="fileNameFormat">
<span class="label-body">Episode file name format:</span>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the text field where the format is entered. Ideally this would be less direct and populated by dragging tokens in to the field or by selection from a menu, but it's a start at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a bit of Javascript to help populate the format field when the format keys are clicked.

Query string
Name string
Condition []string
Query []string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a Condition string array and turned Query in to a string array to support skipping a migration if it's not necessary, and to support multiple Condition and Query statements.

// sqlite3 v3.35.0 supports DROP COLUMN
// "ALTER TABLE settings DROP COLUMN append_episode_number_to_file_name",
// "ALTER TABLE settings DROP COLUMN append_date_to_file_name",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ConvertFileNameFormat needs conditions to ensure it doesn't try to convert the old "append" fields to the new "file name format" if those fields don't exist in the database. Ideally multiple Query statements would be used to drop the old "append" columns, but that's not supported until sqlite3 v3.35. The alternative is recreating the table, but that seems excessive.
Are there other ways to accomplish this?

}
shouldMigrate = shouldMigrate && rawResult == "1"
}
if shouldMigrate {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only execute the migration if the Conditions succeeded.

DB.Save(&Migration{
Date: time.Now(),
Name: name,
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still save the migration to the Migration table as long as there wasn't an actual error.

cleanFileName(podcastName),
fmt.Sprintf("%s%s", episodePathName, fileExtension))
dir, _ := path.Split(finalPath)
createPreSanitizedPath(dir)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly use the episodePathName as it is passed in to Download() and don't sanitize the path components again as they were already sanitized as they were created.

@akhilrex
Copy link
Owner

akhilrex commented Jun 8, 2021

I am a little tied up with work. A quick look into the changes seem to be fine. I would still want to test this locally once. Thanks for the work. Give me some time.

@fracai
Copy link
Contributor Author

fracai commented Jun 8, 2021

No problem. Let me know if there's anything I can do to help.

@akhilrex
Copy link
Owner

akhilrex commented Jun 8, 2021 via email

@fracai
Copy link
Contributor Author

fracai commented Jun 8, 2021

Ah, I considered that, didn't see an obvious way to specify the number of zeros, and figured I'd leave it for another branch. I think it'd be easy enough to use something like %EpisodeNumber:##% to specify the number of zeroes. I'll take a look.

@fracai
Copy link
Contributor Author

fracai commented Jun 9, 2021

Just pushed another commit with support for setting a minimum width for the episode number using %EpisodeNumber:X% to set X minimum digits. If the argument is missing or not parseable as a number, it'll default to no padding.

@fracai
Copy link
Contributor Author

fracai commented Jun 9, 2021

That most recent commit should close #95 as well.

Resolved Conflicts:
	controllers/pages.go
	controllers/podcast.go
	db/podcast.go
	service/podcastService.go
@fracai
Copy link
Contributor Author

fracai commented Jul 24, 2021

Updated to resolve conflicts

@fracai
Copy link
Contributor Author

fracai commented Aug 29, 2021

Resolved conflicts

@treyturner
Copy link

I built this branch, tested it, and it works. I appreciate the additional flexibility in naming, particularly the left-padded episode numbers. Note that I didn't attempt to merge upstream when building/testing though. Would be cool to see this get merged up.

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.

Rename File Based on Format
3 participants