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

Chapter import: Adding support for chapterdb.org XML and TXT formats #42

Merged
merged 3 commits into from Nov 26, 2015

Conversation

sverrirs
Copy link
Contributor

Adding basic support for chapter name import from XML and TXT files downloaded from chapterdb.org. -Only chapter names will be imported into HandBrake and timings and chapter count originally detected by HandBrake will be left unchanged.

Summary:

  • Added basic validation for main import function that validates input data against chapter information retrieved from source media
  • Added two new input parsers for XML and TXT formats (parser is chosen based on file extension)
  • Added a new convenience constructor to GeneralApplicationException to use when thrown with no inner exception

…inor refactorings to accomodate the parsing of the new input formats.
@sverrirs
Copy link
Contributor Author

Whenever you have some spare time @sr55 ;)

@sr55
Copy link
Contributor

sr55 commented Nov 26, 2015

@sverrirs - Sure, Another weekend task for me. Thank you.

if (importedChapters.Count != this.Task.ChapterNames.Count)
{
if (DialogResult.Yes !=
MessageBox.Show(
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a quick note, I have a wrapper around message box in the error service (if i recall correctly). Handy if I ever decide to write a unit test suite for the UI since it allows the mocking of UI responses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent, I was actually going to ask you about that. I will update this to use the error service msgbox :)

@sverrirs
Copy link
Contributor Author

@sr55 Regarding unit-tests I was going to mention that we might want to abstract away all the FileIO access into a IFileInputService of sorts and just pass in Streams into the ViewModel functions. This would make it easier to add tests. I'm more than willing to do the work and add unit-tests for the changes I made to the ChaptersViewModel to kick this effort off :)

sr55 added a commit that referenced this pull request Nov 26, 2015
Chapter import: Adding support for chapterdb.org XML and TXT formats
@sr55 sr55 merged commit 46e641c into HandBrake:master Nov 26, 2015
@sr55
Copy link
Contributor

sr55 commented Nov 26, 2015

If your interested, feel free.

I haven't really put a lot of thought into what I wanted to do with Unit testing yet.
I've typically used NUnit and Moq in the past as that integrates well with ReSharper. I've heard good things about XUnit but not had a chance to actually play with it. The ViewModel / Models and the factories are probably going to be the biggest wins.

If you have any thoughts or suggestions, I'd love to hear.

@sr55
Copy link
Contributor

sr55 commented Nov 26, 2015

P.s If your ever bored, most the developers are on #Handbrake on FreeNode IRC. Various timezones since we are pretty spread out, so if you don't get an response, stick around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants