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.

5 files changed, 162 insertions, 62 deletions

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 @@
     <Reference Include="GongSolutions.Wpf.DragDrop">

       <HintPath>..\libraries\WPFDragDrop\GongSolutions.Wpf.DragDrop.dll</HintPath>

     </Reference>

-    <Reference Include="LumenWorks.Framework.IO">

-      <HintPath>..\libraries\CsvReader\LumenWorks.Framework.IO.dll</HintPath>

-    </Reference>

     <Reference Include="Microsoft.CSharp" />

+    <Reference Include="Microsoft.VisualBasic" />

     <Reference Include="Newtonsoft.Json, Version=4.5.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">

       <SpecificVersion>False</SpecificVersion>

       <HintPath>..\libraries\json\Newtonsoft.Json.dll</HintPath>

@@ -234,6 +232,7 @@
     <Compile Include="Utilities\GeneralUtilities.cs" />

     <Compile Include="Utilities\HandBrakeApp.cs" />

     <Compile Include="Utilities\Interfaces\INotifyPropertyChangedEx.cs" />

+    <Compile Include="Utilities\Output\CsvHelper.cs" />

     <Compile Include="Utilities\PropertyChangedBase.cs" />

     <Compile Include="Utilities\Win7.cs" />

     <Compile Include="ViewModels\CountdownAlertViewModel.cs" />

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 {
         }

         

         /// <summary>

-        ///   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! .

         /// </summary>

         public static string ChaptersViewModel_UnableToExportChaptersWarning {

             get {

@@ -457,6 +457,42 @@ namespace HandBrakeWPF.Properties {
         }

         

         /// <summary>

+        ///   Looks up a localized string similar to First column in chapters file must only contain a integer number value higher than zero (0).

+        /// </summary>

+        public static string ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber {

+            get {

+                return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber", resourceCulture);

+            }

+        }

+        

+        /// <summary>

+        ///   Looks up a localized string similar to All lines in chapters file must have at least 2 columns of data.

+        /// </summary>

+        public static string ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns {

+            get {

+                return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns", resourceCulture);

+            }

+        }

+        

+        /// <summary>

+        ///   Looks up a localized string similar to Line {0} is invalid. Nothing will be imported..

+        /// </summary>

+        public static string ChaptersViewModel_UnableToImportChaptersMalformedLineMsg {

+            get {

+                return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersMalformedLineMsg", resourceCulture);

+            }

+        }

+        

+        /// <summary>

+        ///   Looks up a localized string similar to Unable to import chapter file.

+        /// </summary>

+        public static string ChaptersViewModel_UnableToImportChaptersWarning {

+            get {

+                return ResourceManager.GetString("ChaptersViewModel_UnableToImportChaptersWarning", resourceCulture);

+            }

+        }

+        

+        /// <summary>

         ///   Looks up a localized string similar to Confirm.

         /// </summary>

         public static string 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.</value>
     <value>Switch Back To Tracks</value>

   </data>

   <data name="ChaptersViewModel_UnableToExportChaptersWarning" xml:space="preserve">

-    <value>Unable to save Chapter Makrers file! </value>

+    <value>Unable to save Chapter Markers file! </value>

   </data>

   <data name="ChaptersViewModel_UnableToExportChaptersMsg" xml:space="preserve">

     <value>Chapter marker names will NOT be saved in your encode.</value>

@@ -739,4 +739,16 @@ Your old presets file was archived to:</value>
   <data name="MainViewModel_LowDiskSpaceWarning" xml:space="preserve">

     <value>Warning, you are running low on disk space. HandBrake will not be able to complete this encode if you run out of space. </value>

   </data>

+  <data name="ChaptersViewModel_UnableToImportChaptersMalformedLineMsg" xml:space="preserve">

+    <value>Line {0} is invalid. Nothing will be imported.</value>

+  </data>

+  <data name="ChaptersViewModel_UnableToImportChaptersWarning" xml:space="preserve">

+    <value>Unable to import chapter file</value>

+  </data>

+  <data name="ChaptersViewModel_UnableToImportChaptersLineDoesNotHaveAtLeastTwoColumns" xml:space="preserve">

+    <value>All lines in chapters file must have at least 2 columns of data</value>

+  </data>

+  <data name="ChaptersViewModel_UnableToImportChaptersFirstColumnMustContainOnlyIntegerNumber" xml:space="preserve">

+    <value>First column in chapters file must only contain a integer number value higher than zero (0)</value>

+  </data>

 </root>
\ 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
+{
+    /// <summary>
+    /// Utilitiy functions for writing CSV files
+    /// </summary>
+    internal sealed class CsvHelper
+    {
+        private const string QUOTE = "\"";
+        private const string ESCAPED_QUOTE = "\"\"";
+        private static readonly char[] CHARACTERS_THAT_MUST_BE_QUOTED = { ',', '"', '\n' };
+
+        /// <summary>
+        /// Properly escapes a string value containing reserved characters with double quotes "..." before it is written to a CSV file.
+        /// </summary>
+        /// <param name="value">Value to be escaped</param>
+        /// <returns>Fully escaped value</returns>
+        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;

@@ -94,49 +95,39 @@ namespace HandBrakeWPF.ViewModels
         #region Public Methods

 

         /// <summary>

-        /// Export a CSV file.

-        /// </summary>

-        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);

-            }

-        }

-

-        /// <summary>

         /// Export the Chapter Markers to a CSV file

         /// </summary>

-        /// <param name="filename">

-        /// The filename.

-        /// </param>

         /// <exception cref="GeneralApplicationException">

         /// Thrown when exporting fails.

         /// </exception>

-        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
         /// </summary>

         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<int, string> chapterMap = new Dictionary<int, string>();

-            try

+            var chapterMap = new Dictionary<int, string>();

+

+            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;

             }

         }