File renaming: suppress duplicate suffixes #3317 (#4389)

chrome/content/zotero/xpcom/attachments.js

@@ -2152,6 +2152,9 @@ Zotero.Attachments = new function () {
 			formatString = Zotero.Prefs.get('attachmentRenameTemplate');
 		}
 
+		let chunks = [];
+		let protectedLiterals = new Set();
+
 		formatString = formatString.trim();
 
 		const getSlicedCreatorsOfType = (creatorType, slice) => {
@@ -2190,18 +2193,45 @@ Zotero.Attachments = new function () {
 			if (value === '' || value === null || typeof value === 'undefined') {
 				return '';
 			}
+
+			if (prefix === '\\' || prefix === '/') {
+				prefix = '';
+			}
+
+			if (suffix === '\\' || suffix === '/') {
+				suffix = '';
+			}
+
+			if (protectedLiterals.size > 0) {
+				// escape protected literals in the format string with \
+				value = value.replace(
+					new RegExp(`(${Array.from(protectedLiterals.keys()).join('|')})`, 'g'),
+					'\\$1//'
+				);
+			}
+
 			if (truncate) {
 				value = value.substr(0, truncate);
 			}
 
 			value = value.trim();
+			let rawValue = value;
 
-			if (prefix) {
+			let affixed = false;
+
+			if (prefix && !value.startsWith(prefix)) {
 				value = prefix + value;
+				affixed = true;
 			}
-			if (suffix) {
+			if (suffix && !value.endsWith(suffix)) {
 				value += suffix;
+				affixed = true;
 			}
+
+			if (affixed) {
+				chunks.push({ value, rawValue, suffix, prefix });
+			}
+
 			switch (textCase) {
 				case 'upper':
 					value = value.toUpperCase();
@@ -2297,10 +2327,44 @@ Zotero.Attachments = new function () {
 
 		const vars = { ...fields, ...creatorFields, firstCreator, itemType, year };
 
-		formatString = Zotero.Utilities.Internal.generateHTMLFromTemplate(formatString, vars);
-		formatString = Zotero.Utilities.cleanTags(formatString);
-		formatString = Zotero.File.getValidFileName(formatString);
-		return formatString;
+
+		// Final name is generated twice. In the first pass we collect all affixed values and determine protected literals.
+		// This is done in order to remove repeated suffixes, except if these appear in the value or the format string itself.
+		// See "should suppress suffixes where they would create a repeat character" test for edge cases.
+		let formatted = Zotero.Utilities.Internal.generateHTMLFromTemplate(formatString, vars);
+		
+		let replacePairs = new Map();
+		for (let chunk of chunks) {
+			if (chunk.suffix && formatted.includes(`${chunk.rawValue}${chunk.suffix}${chunk.suffix}`)) {
+				protectedLiterals.add(`${chunk.rawValue}${chunk.suffix}${chunk.suffix}`);
+				replacePairs.set(`${chunk.rawValue}${chunk.suffix}${chunk.suffix}`, `${chunk.rawValue}${chunk.suffix}`);
+			}
+			if (chunk.prefix && formatted.includes(`${chunk.prefix}${chunk.prefix}${chunk.rawValue}`)) {
+				protectedLiterals.add(`${chunk.prefix}${chunk.prefix}${chunk.rawValue}`);
+				replacePairs.set(`${chunk.prefix}${chunk.prefix}${chunk.rawValue}`, `${chunk.prefix}${chunk.rawValue}`);
+			}
+		}
+
+		// Use "/" and "\" as escape characters for protected literals. We need two different escape chars for edge cases.
+		// Both escape chars are invalid in file names and thus removed from the final string by `getValidFileName`
+		if (protectedLiterals.size > 0) {
+			formatString = formatString.replace(
+				new RegExp(`(${Array.from(protectedLiterals.keys()).join('|')})`, 'g'),
+				'\\$1//'
+			);
+		}
+
+		formatted = Zotero.Utilities.Internal.generateHTMLFromTemplate(formatString, vars);
+		if (replacePairs.size > 0) {
+			formatted = formatted.replace(
+				new RegExp(`(${Array.from(replacePairs.keys()).map(replace => `(?<!\\\\)${replace}(?!//)`).join('|')})`, 'g'),
+				match => replacePairs.get(match)
+			);
+		}
+		
+		formatted = Zotero.Utilities.cleanTags(formatted);
+		formatted = Zotero.File.getValidFileName(formatted);
+		return formatted;
 	};
 	
 	

test/tests/attachmentsTest.js

@@ -1334,7 +1334,7 @@ describe("Zotero.Attachments", function() {
 	});
 	
 	describe("#getFileBaseNameFromItem()", function () {
-		var item, itemManyAuthors, itemPatent, itemIncomplete, itemBookSection, itemSpaces;
+		var item, itemManyAuthors, itemPatent, itemIncomplete, itemBookSection, itemSpaces, itemSuffixes, itemKeepDashes;
 
 		before(() => {
 			item = createUnsavedDataObject('item', { title: 'Lorem Ipsum', itemType: 'journalArticle' });
@@ -1376,6 +1376,12 @@ describe("Zotero.Attachments", function() {
 			itemBookSection = createUnsavedDataObject('item', { title: 'Book Section', itemType: 'bookSection' });
 			itemBookSection.setField('bookTitle', 'Book Title');
 			itemSpaces = createUnsavedDataObject('item', { title: ' Spaces! ', itemType: 'book' });
+			itemSuffixes = createUnsavedDataObject('item', { title: '-Suffixes-', itemType: 'book' });
+			itemSuffixes.setField('date', "1999-07-15");
+			itemKeepDashes = createUnsavedDataObject('item', { title: 'keep--dashes', itemType: 'journalArticle' });
+			itemKeepDashes.setField('publicationTitle', "keep");
+			itemKeepDashes.setField('issue', 'dashes');
+			itemKeepDashes.setField('date', "1999-07-15");
 		});
 
 		
@@ -1619,6 +1625,43 @@ describe("Zotero.Attachments", function() {
 			);
 		});
 
+		it("should suppress suffixes where they would create a repeat character", function () {
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(item, '{{ title suffix="-" }}{{ year prefix="-" }}'),
+				'Lorem Ipsum-1975'
+			);
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemSuffixes, '{{ title prefix="-" suffix="-" }}{{ year }}'),
+				'-Suffixes-1999'
+			);
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemSuffixes, '{{ title suffix="-" }}{{ year prefix="-" }}'),
+				'-Suffixes-1999'
+			);
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemKeepDashes, '{{ title suffix="-" }}{{ year prefix="-" }}'),
+				'keep--dashes-1999'
+			);
+			// keep--dashes is a title and should be kept unchanged but "keep" and "dashes" are fields
+			// separated by prefixes and suffixes where repeated characters should be suppressed
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemKeepDashes, '{{ title suffix="-" }}{{ publicationTitle suffix="-" }}{{ issue prefix="-" }}'),
+				'keep--dashes-keep-dashes'
+			);
+			// keep--dashes is provided as literal part of the template and should be kept unchanged
+			// but "keep" and "dashes" are fields separated by prefixes and suffixes where repeated
+			// characters should be suppressed. Finally "keep--dashes" title is appended at the end
+			// which should also be kept as is.
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemKeepDashes, 'keep--dashes-{{ publicationTitle prefix="-" suffix="-" }}{{ issue prefix="-" suffix="-" }}-keep--dashes-{{ publicationTitle suffix="-" }}test{{ title prefix="-" }}'),
+				'keep--dashes-keep-dashes-keep--dashes-keep-test-keep--dashes'
+			);
+			assert.equal(
+				Zotero.Attachments.getFileBaseNameFromItem(itemSuffixes, '{{ title prefix="/" suffix="\\" }}{{ year }}'),
+				'-Suffixes-1999'
+			);
+		});
+
 		it("should convert old attachmentRenameFormatString to use new attachmentRenameTemplate syntax", function () {
 			assert.equal(
 				Zotero.Prefs.convertLegacyAttachmentRenameFormatString('{%c - }{%y - }{%t{50}}'),