From 9e107ee850e396044c4c80c3257a4623995b88b3 Mon Sep 17 00:00:00 2001 From: Sverrir Sigmundarson Date: Wed, 18 Nov 2015 23:57:03 +0100 Subject: Fixing importing and exporting of chapters via CSV files. Adding proper handling of escape characters, handling of most common alternative value separators. Fixing resource leakage via undisposed FileDialogs. --- win/CS/HandBrakeWPF/HandBrakeWPF.csproj | 5 +- .../HandBrakeWPF/Properties/Resources.Designer.cs | 38 +++++- win/CS/HandBrakeWPF/Properties/Resources.resx | 14 ++- win/CS/HandBrakeWPF/Utilities/Output/CsvHelper.cs | 34 ++++++ .../HandBrakeWPF/ViewModels/ChaptersViewModel.cs | 133 ++++++++++++--------- 5 files changed, 162 insertions(+), 62 deletions(-) create mode 100644 win/CS/HandBrakeWPF/Utilities/Output/CsvHelper.cs (limited to 'win') diff --git a/win/CS/HandBrakeWPF/HandBrakeWPF.csproj b/win/CS/HandBrakeWPF/HandBrakeWPF.csproj index 2af3ba412..00b0560e9 100644 --- a/win/CS/HandBrakeWPF/HandBrakeWPF.csproj +++ b/win/CS/HandBrakeWPF/HandBrakeWPF.csproj @@ -87,10 +87,8 @@ ..\libraries\WPFDragDrop\GongSolutions.Wpf.DragDrop.dll - - ..\libraries\CsvReader\LumenWorks.Framework.IO.dll - + False ..\libraries\json\Newtonsoft.Json.dll @@ -234,6 +232,7 @@ + diff --git a/win/CS/HandBrakeWPF/Properties/Resources.Designer.cs b/win/CS/HandBrakeWPF/Properties/Resources.Designer.cs index 643843132..b53620537 100644 --- a/win/CS/HandBrakeWPF/Properties/Resources.Designer.cs +++ b/win/CS/HandBrakeWPF/Properties/Resources.Designer.cs @@ -448,7 +448,7 @@ namespace HandBrakeWPF.Properties { } /// - /// Looks up a localized string similar to Unable to save Chapter Makrers file! . + /// Looks up a localized string similar to Unable to save Chapter Markers file! . /// public static string ChaptersViewModel_UnableToExportChaptersWarning { get { @@ -456,6 +456,42 @@ namespace HandBrakeWPF.Properties { } } + /// + /// Looks up a localized string similar to First column in chapters file must only contain a integer number value higher than zero (0). + /// + public static string ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber { + get { + return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to All lines in chapters file must have at least 2 columns of data. + /// + public static string ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns { + get { + return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Line {0} is invalid. Nothing will be imported.. + /// + public static string ChaptersViewModel_UnableToImportChaptersMalformedLineMsg { + get { + return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersMalformedLineMsg", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unable to import chapter file. + /// + public static string ChaptersViewModel_UnableToImportChaptersWarning { + get { + return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersWarning", resourceCulture); + } + } + /// /// Looks up a localized string similar to Confirm. /// diff --git a/win/CS/HandBrakeWPF/Properties/Resources.resx b/win/CS/HandBrakeWPF/Properties/Resources.resx index 0ce417b62..a8b2cb469 100644 --- a/win/CS/HandBrakeWPF/Properties/Resources.resx +++ b/win/CS/HandBrakeWPF/Properties/Resources.resx @@ -584,7 +584,7 @@ The Activity log may have further information. Switch Back To Tracks - Unable to save Chapter Makrers file! + Unable to save Chapter Markers file! Chapter marker names will NOT be saved in your encode. @@ -739,4 +739,16 @@ Your old presets file was archived to: Warning, you are running low on disk space. HandBrake will not be able to complete this encode if you run out of space. + + Line {0} is invalid. Nothing will be imported. + + + Unable to import chapter file + + + All lines in chapters file must have at least 2 columns of data + + + First column in chapters file must only contain a integer number value higher than zero (0) + \ No newline at end of file diff --git a/win/CS/HandBrakeWPF/Utilities/Output/CsvHelper.cs b/win/CS/HandBrakeWPF/Utilities/Output/CsvHelper.cs new file mode 100644 index 000000000..cf2d7a38d --- /dev/null +++ b/win/CS/HandBrakeWPF/Utilities/Output/CsvHelper.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace HandBrakeWPF.Utilities.Output +{ + /// + /// Utilitiy functions for writing CSV files + /// + internal sealed class CsvHelper + { + private const string QUOTE = "\""; + private const string ESCAPED_QUOTE = "\"\""; + private static readonly char[] CHARACTERS_THAT_MUST_BE_QUOTED = { ',', '"', '\n' }; + + /// + /// Properly escapes a string value containing reserved characters with double quotes "..." before it is written to a CSV file. + /// + /// Value to be escaped + /// Fully escaped value + public static string Escape(string value) + { + if (value.Contains(QUOTE)) + value = value.Replace(QUOTE, ESCAPED_QUOTE); + + if (value.IndexOfAny(CHARACTERS_THAT_MUST_BE_QUOTED) > -1) + value = QUOTE + value + QUOTE; + + return value; + } + } +} diff --git a/win/CS/HandBrakeWPF/ViewModels/ChaptersViewModel.cs b/win/CS/HandBrakeWPF/ViewModels/ChaptersViewModel.cs index a76d24547..4815de20d 100644 --- a/win/CS/HandBrakeWPF/ViewModels/ChaptersViewModel.cs +++ b/win/CS/HandBrakeWPF/ViewModels/ChaptersViewModel.cs @@ -13,6 +13,8 @@ namespace HandBrakeWPF.ViewModels using System.Collections.Generic; using System.Collections.ObjectModel; using System.IO; + using System.Text; + using System.Windows.Forms; using Caliburn.Micro; @@ -20,11 +22,10 @@ namespace HandBrakeWPF.ViewModels using HandBrakeWPF.Services.Interfaces; using HandBrakeWPF.Services.Presets.Model; using HandBrakeWPF.Services.Scan.Model; + using HandBrakeWPF.Utilities.Output; using HandBrakeWPF.ViewModels.Interfaces; - using LumenWorks.Framework.IO.Csv; - - using Microsoft.Win32; + using Microsoft.VisualBasic.FileIO; using ChapterMarker = HandBrakeWPF.Services.Encode.Model.Models.ChapterMarker; using EncodeTask = HandBrakeWPF.Services.Encode.Model.EncodeTask; @@ -93,50 +94,40 @@ namespace HandBrakeWPF.ViewModels #region Public Methods - /// - /// Export a CSV file. - /// - public void Export() - { - var saveFileDialog = new OpenFileDialog - { - Filter = "Csv File|*.csv", - DefaultExt = "csv", - CheckPathExists = true - }; - bool? dialogResult = saveFileDialog.ShowDialog(); - if (dialogResult.HasValue && dialogResult.Value && !string.IsNullOrEmpty(saveFileDialog.FileName)) - { - this.ExportChaptersToCSV(saveFileDialog.FileName); - } - } - /// /// Export the Chapter Markers to a CSV file /// - /// - /// The filename. - /// /// /// Thrown when exporting fails. /// - public void ExportChaptersToCSV(string filename) + public void Export() { - try + string fileName = null; + using (var saveFileDialog = new SaveFileDialog() + { + Filter = "Csv File|*.csv", + DefaultExt = "csv", + CheckPathExists = true, + OverwritePrompt = true + }) { - string csv = string.Empty; + var dialogResult = saveFileDialog.ShowDialog(); + fileName = saveFileDialog.FileName; - foreach (ChapterMarker row in this.Task.ChapterNames) + // Exit early if the user cancelled or the filename is invalid + if (dialogResult != DialogResult.OK || string.IsNullOrWhiteSpace(fileName)) + return; + } + + try + { + using (var csv = new StreamWriter(fileName)) { - csv += row.ChapterNumber.ToString(); - csv += ","; - csv += row.ChapterName.Replace(",", "\\,"); - csv += Environment.NewLine; + foreach (ChapterMarker row in this.Task.ChapterNames) + { + csv.Write("{0},{1}{2}", row.ChapterNumber, CsvHelper.Escape(row.ChapterName), Environment.NewLine); + } } - var file = new StreamWriter(filename); - file.Write(csv); - file.Close(); - file.Dispose(); } catch (Exception exc) { @@ -152,41 +143,69 @@ namespace HandBrakeWPF.ViewModels /// public void Import() { - var dialog = new OpenFileDialog { Filter = "CSV files (*.csv)|*.csv", CheckFileExists = true }; - bool? dialogResult = dialog.ShowDialog(); - string filename = dialog.FileName; - - if (!dialogResult.HasValue || !dialogResult.Value || string.IsNullOrEmpty(filename)) + string filename = null; + using (var dialog = new OpenFileDialog() { Filter = "CSV files (*.csv)|*.csv", CheckFileExists = true }) { - return; + var dialogResult = dialog.ShowDialog(); + filename = dialog.FileName; + + // Exit if the user didn't press the OK button or the file name is invalid + if (dialogResult != DialogResult.OK || string.IsNullOrWhiteSpace(filename)) + return; } - IDictionary chapterMap = new Dictionary(); - try + var chapterMap = new Dictionary(); + + using (TextFieldParser csv = new TextFieldParser(filename) + { CommentTokens = new[] { "#" }, // Comment lines + Delimiters = new[] { ",", "\t", ";", ":" }, // Support all of these common delimeter types + HasFieldsEnclosedInQuotes = true, // Assume that our data will be properly escaped + TextFieldType = FieldType.Delimited, + TrimWhiteSpace = true // Remove excess whitespace from ends of imported values + }) { - using (CsvReader csv = new CsvReader(new StreamReader(filename), false)) + while (!csv.EndOfData) { - while (csv.ReadNextRecord()) + try { - if (csv.FieldCount == 2) - { - int chapter; - int.TryParse(csv[0], out chapter); - chapterMap[chapter] = csv[1]; - } + // Only read the first two columns, anything else will be ignored but will not raise an error + var row = csv.ReadFields(); + if (row == null || row.Length < 2) // null condition happens if the file is somehow corrupt during reading + throw new MalformedLineException(Resources.ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns, csv.LineNumber); + + int chapterNumber; + if (!int.TryParse(row[0], out chapterNumber)) + throw new MalformedLineException(Resources.ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber, csv.LineNumber); + + // Store the chapter name at the correct index + chapterMap[chapterNumber] = row[1]?.Trim(); + } + catch (MalformedLineException mlex) + { + throw new GeneralApplicationException( + Resources.ChaptersViewModel_UnableToImportChaptersWarning, + string.Format(Resources.ChaptersViewModel_UnableToImportChaptersMalformedLineMsg, mlex.LineNumber), + mlex); } } } - catch (Exception) - { - // Do Nothing - } + + // Exit early if no chapter information was extracted + if (chapterMap.Count <= 0) + return; // Now iterate over each chatper we have, and set it's name foreach (ChapterMarker item in this.Task.ChapterNames) { string chapterName; - item.ChapterName = chapterMap.TryGetValue(item.ChapterNumber, out chapterName) ? chapterName : string.Empty; + + // If we don't have a chapter name for this chapter then + // fallback to the value that is already set for the chapter + if (!chapterMap.TryGetValue(item.ChapterNumber, out chapterName)) + chapterName = item.ChapterName; + + // Assign the chapter name unless the name is not set or only whitespace charaters + item.ChapterName = !string.IsNullOrWhiteSpace(chapterName) ? chapterName : string.Empty; } } -- cgit v1.2.3