From c7a2cca7b435949352de290627bad53e889d6cd0 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 22 Oct 2023 22:40:40 -0700 Subject: [PATCH 1/8] Add a test that demonstrates the issue --- .../Automation/CommandLineIntegrationTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index bf8e8bb26..bb1180cdf 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -282,6 +282,40 @@ public class CommandLineIntegrationTests : ContextualTests AssertRecoveryCleanedUp(); } + [Fact] + public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() + { + var path = $"{FolderPath}/test.pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); + Assert.False(File.Exists($"{FolderPath}/test.4.pdf")); + Assert.False(File.Exists($"{FolderPath}/test.5.pdf")); + Assert.False(File.Exists($"{FolderPath}/test.6.pdf")); + AssertRecoveryCleanedUp(); + } + [Fact] public async Task MultipleImages() { From fb1a110c7668f361dc1874414fcfd0b644d250ce Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 22 Oct 2023 22:44:57 -0700 Subject: [PATCH 2/8] Utilize the force overwrite option during placeholder substitution --- NAPS2.Lib/Automation/AutomatedScanning.cs | 2 +- NAPS2.Lib/Pdf/SavePdfOperation.cs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/NAPS2.Lib/Automation/AutomatedScanning.cs b/NAPS2.Lib/Automation/AutomatedScanning.cs index f9f7c389f..38dff6285 100644 --- a/NAPS2.Lib/Automation/AutomatedScanning.cs +++ b/NAPS2.Lib/Automation/AutomatedScanning.cs @@ -585,7 +585,7 @@ public class AutomatedScanning } }; int digits = (int) Math.Floor(Math.Log10(_scanList.Count)) + 1; - string actualPath = _placeholders.Substitute(path, true, scanIndex++, _scanList.Count > 1 ? digits : 0); + string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, scanIndex++, _scanList.Count > 1 ? digits : 0); op.Start(actualPath, _placeholders, fileContents, _config.Get(c => c.PdfSettings), _ocrParams); if (!await op.Success) { diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index e149b1c3d..110150330 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -72,8 +72,9 @@ internal class SavePdfOperation : OperationBase int i = 0; foreach (var imagesForFile in imagesByFile) { - var currentFileName = placeholders.Substitute(fileName, true, i, singleFile ? 0 : digits); - // TODO: Overwrite prompt non-single file? + var currentFileName = placeholders.Substitute(fileName, + _overwritePrompt.ConfirmOverwrite(fileName) == OverwriteResponse.Yes, + i, singleFile ? 0 : digits); Status.StatusText = string.Format(MiscResources.SavingFormat, Path.GetFileName(currentFileName)); InvokeStatusChanged(); if (singleFile && IsFileInUse(currentFileName, out var ex)) From 5b8c6b5d3f9f29e3dfd0ab15f0ab3dca5ab70d03 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 22 Oct 2023 23:36:37 -0700 Subject: [PATCH 3/8] Add the NoOverwrite test --- .../Automation/CommandLineIntegrationTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index bb1180cdf..5bac758ae 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -282,6 +282,38 @@ public class CommandLineIntegrationTests : ContextualTests AssertRecoveryCleanedUp(); } + [Fact] + public async Task ExistingFile_NoOverwrite_SplitWithNoPlaceholder() + { + var path = $"{FolderPath}/test.pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); + Assert.True(File.Exists($"{FolderPath}/test.4.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.5.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.6.pdf")); + AssertRecoveryCleanedUp(); + } + [Fact] public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() { From 4d65b4bcd7adad57e0bccb676bb08f50364944f8 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 23 Oct 2023 18:09:04 -0700 Subject: [PATCH 4/8] Remove confirm overwrite from PDF save --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 110150330..776f4eb93 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -70,11 +70,10 @@ internal class SavePdfOperation : OperationBase { int digits = (int) Math.Floor(Math.Log10(images.Count)) + 1; int i = 0; + foreach (var imagesForFile in imagesByFile) { - var currentFileName = placeholders.Substitute(fileName, - _overwritePrompt.ConfirmOverwrite(fileName) == OverwriteResponse.Yes, - i, singleFile ? 0 : digits); + var currentFileName = placeholders.Substitute(fileName, true, i, singleFile ? 0 : digits); Status.StatusText = string.Format(MiscResources.SavingFormat, Path.GetFileName(currentFileName)); InvokeStatusChanged(); if (singleFile && IsFileInUse(currentFileName, out var ex)) From 72ab085824cf890aa663204e17b0f8b261bbf92d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:42:49 -0700 Subject: [PATCH 5/8] Add WithPlaceholder tests --- .../Automation/CommandLineIntegrationTests.cs | 116 +++++++++++++++--- 1 file changed, 97 insertions(+), 19 deletions(-) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index 5bac758ae..bf6ef1e79 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -282,6 +282,78 @@ public class CommandLineIntegrationTests : ContextualTests AssertRecoveryCleanedUp(); } + [Fact] + public async Task ExistingFile_NoOverwrite_SplitWithPlaceholder() + { + var path = $"{FolderPath}/test$(n).pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + PdfAsserts.AssertImages($"{FolderPath}/test1.pdf", Image1); + PdfAsserts.AssertImages($"{FolderPath}/test2.pdf", Image2); + PdfAsserts.AssertImages($"{FolderPath}/test3.pdf", Image3); + Assert.True(File.Exists($"{FolderPath}/test4.pdf")); + Assert.True(File.Exists($"{FolderPath}/test5.pdf")); + Assert.True(File.Exists($"{FolderPath}/test6.pdf")); + AssertRecoveryCleanedUp(); + } + + [Fact] + public async Task ExistingFile_ForceOverwrite_SplitWithPlaceholder() + { + var path = $"{FolderPath}/test$(n).pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + PdfAsserts.AssertImages($"{FolderPath}/test1.pdf", Image1); + PdfAsserts.AssertImages($"{FolderPath}/test2.pdf", Image2); + PdfAsserts.AssertImages($"{FolderPath}/test3.pdf", Image3); + Assert.False(File.Exists($"{FolderPath}/test4.pdf")); + Assert.False(File.Exists($"{FolderPath}/test5.pdf")); + Assert.False(File.Exists($"{FolderPath}/test6.pdf")); + AssertRecoveryCleanedUp(); + } + [Fact] public async Task ExistingFile_NoOverwrite_SplitWithNoPlaceholder() { @@ -295,16 +367,19 @@ public class CommandLineIntegrationTests : ContextualTests Verbose = true }, new[] { Image1, Image2, Image3 }); - await _automationHelper.RunCommand( - new AutomatedScanningOptions + await _automationHelper.WithContainer(container => { - OutputPath = path, - ProfileName = string.Empty, - Split = true, - Verbose = true - }, - new[] { Image1, Image2, Image3 }); - + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); @@ -328,17 +403,20 @@ public class CommandLineIntegrationTests : ContextualTests Verbose = true }, new[] { Image1, Image2, Image3 }); - await _automationHelper.RunCommand( - new AutomatedScanningOptions + await _automationHelper.WithContainer(container => { - OutputPath = path, - ForceOverwrite = true, - ProfileName = string.Empty, - Split = true, - Verbose = true - }, - new[] { Image1, Image2, Image3 }); - + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); From 29ab522b38a06acf474954878f2f99dd589852b9 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:49:50 -0700 Subject: [PATCH 6/8] Add incrementPlaceholderIfExists substitution parameter -Used to determine whether to increment the placeholder number. --- NAPS2.Lib/Automation/AutomatedScanning.cs | 3 ++- NAPS2.Sdk/ImportExport/Placeholders.cs | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/NAPS2.Lib/Automation/AutomatedScanning.cs b/NAPS2.Lib/Automation/AutomatedScanning.cs index 38dff6285..d008044bd 100644 --- a/NAPS2.Lib/Automation/AutomatedScanning.cs +++ b/NAPS2.Lib/Automation/AutomatedScanning.cs @@ -585,7 +585,8 @@ public class AutomatedScanning } }; int digits = (int) Math.Floor(Math.Log10(_scanList.Count)) + 1; - string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, scanIndex++, _scanList.Count > 1 ? digits : 0); + string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, + scanIndex++, _scanList.Count > 1 ? digits : 0, !_options.ForceOverwrite); op.Start(actualPath, _placeholders, fileContents, _config.Get(c => c.PdfSettings), _ocrParams); if (!await op.Success) { diff --git a/NAPS2.Sdk/ImportExport/Placeholders.cs b/NAPS2.Sdk/ImportExport/Placeholders.cs index b0f82bf2a..be5490243 100644 --- a/NAPS2.Sdk/ImportExport/Placeholders.cs +++ b/NAPS2.Sdk/ImportExport/Placeholders.cs @@ -46,21 +46,22 @@ internal abstract class Placeholders /// Whether to use an auto-incrementing file number to make the file name unique. /// The file number will be at least one bigger than this value. /// The minimum number of digits in the file number. Only has an effect if the path does not contain a numeric placeholder like $(n) or $(nnn). + /// Whether to increment the placeholder number to make the file name unique. /// The file path with substitutions. [return: NotNullIfNotNull("filePath")] public abstract string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0); + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true); public class StubPlaceholders : Placeholders { public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) => filePath; + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) => filePath; } public class EnvironmentPlaceholders : Placeholders { public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) { if (filePath == null) return null; return Environment.ExpandEnvironmentVariables(filePath); @@ -99,7 +100,7 @@ internal abstract class Placeholders [return: NotNullIfNotNull("filePath")] public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) { if (filePath == null) { @@ -116,7 +117,7 @@ internal abstract class Placeholders if (match.Success) { result = NumberPlaceholderPattern.Replace(result, ""); - result = SubstituteNumber(result, match.Index, match.Length - 3, numberSkip, true); + result = SubstituteNumber(result, match.Index, match.Length - 3, numberSkip, incrementPlaceholderIfExists); } else if (autoNumberDigits > 0) { From 5344dc519e5a6e889ce04c713a16186ee979a1d1 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:55:50 -0700 Subject: [PATCH 7/8] Include the original todo message --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 776f4eb93..7989ce8b4 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -74,6 +74,7 @@ internal class SavePdfOperation : OperationBase foreach (var imagesForFile in imagesByFile) { var currentFileName = placeholders.Substitute(fileName, true, i, singleFile ? 0 : digits); + // TODO: Overwrite prompt non-single file? Status.StatusText = string.Format(MiscResources.SavingFormat, Path.GetFileName(currentFileName)); InvokeStatusChanged(); if (singleFile && IsFileInUse(currentFileName, out var ex)) From 2b08aae77a040ae28c3863c694e3630899a84c5b Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:57:29 -0700 Subject: [PATCH 8/8] Fix original spacing --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 7989ce8b4..89d15aac5 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -70,7 +70,6 @@ internal class SavePdfOperation : OperationBase { int digits = (int) Math.Floor(Math.Log10(images.Count)) + 1; int i = 0; - foreach (var imagesForFile in imagesByFile) { var currentFileName = placeholders.Substitute(fileName, true, i, singleFile ? 0 : digits);