-
Notifications
You must be signed in to change notification settings - Fork 97
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
base: master
Are you sure you want to change the base?
Episode path format #108
Conversation
…g struct, SettingModel struct, UpdateSetting()
…h for downloaded episodes
…e and append_episode_number to file_name_format
<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> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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", | ||
}, |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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, | ||
}) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
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. |
No problem. Let me know if there's anything I can do to help. |
Since you have already worked on it maybe you could see if #95 can be
accommodated in this as well.
Thanks and Regards
Akhil Gupta
…On Tue, 8 Jun 2021, 6:52 pm Arno Hautala, ***@***.***> wrote:
No problem. Let me know if there's anything I can do to help.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#108 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEMVPRUC772BWQ2PFFHTTTTRYKRFANCNFSM46DBE4LQ>
.
|
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. |
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. |
That most recent commit should close #95 as well. |
Resolved Conflicts: controllers/pages.go controllers/podcast.go db/podcast.go service/podcastService.go
Updated to resolve conflicts |
Resolved conflicts |
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. |
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