From 6ddb3970f7d5b63ca5b39c36685417fb6309701a Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 22 Oct 2024 14:28:48 +0200 Subject: [PATCH] warn if file name is too long for concat/merge closes #2200 --- src/renderer/src/App.tsx | 4 +- src/renderer/src/components/Button.module.css | 5 +++ src/renderer/src/components/ConcatDialog.tsx | 40 +++++++++++++++---- src/renderer/src/util/outputNameTemplate.ts | 16 +++++--- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 3e1b4d11b0..5906c3f11c 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -82,7 +82,7 @@ import { askForOutDir, askForImportChapters, promptTimecode, askForFileOpenActio import { openSendReportDialog } from './reporting'; import { fallbackLng } from './i18n'; import { findSegmentsAtCursor, sortSegments, convertSegmentsToChapters, hasAnySegmentOverlap, isDurationValid, playOnlyCurrentSegment, getSegmentTags } from './segments'; -import { generateOutSegFileNames as generateOutSegFileNamesRaw, generateMergedFileNames as generateMergedFileNamesRaw, defaultOutSegTemplate, defaultMergedFileTemplate } from './util/outputNameTemplate'; +import { generateOutSegFileNames as generateOutSegFileNamesRaw, generateMergedFileNames as generateMergedFileNamesRaw, defaultOutSegTemplate, defaultCutMergedFileTemplate } from './util/outputNameTemplate'; import { rightBarWidth, leftBarWidth, ffmpegExtractWindow, zoomMax } from './util/constants'; import BigWaveform from './components/BigWaveform'; @@ -193,7 +193,7 @@ function App() { }, [customFfPath]); const outSegTemplateOrDefault = outSegTemplate || defaultOutSegTemplate; - const mergedFileTemplateOrDefault = mergedFileTemplate || defaultMergedFileTemplate; + const mergedFileTemplateOrDefault = mergedFileTemplate || defaultCutMergedFileTemplate; useEffect(() => { const l = language || fallbackLng; diff --git a/src/renderer/src/components/Button.module.css b/src/renderer/src/components/Button.module.css index 977251c1ee..f683b3566b 100644 --- a/src/renderer/src/components/Button.module.css +++ b/src/renderer/src/components/Button.module.css @@ -11,3 +11,8 @@ border: .05em solid var(--gray7); cursor: pointer; } + +.button:disabled { + opacity: .5; + cursor: initial; +} \ No newline at end of file diff --git a/src/renderer/src/components/ConcatDialog.tsx b/src/renderer/src/components/ConcatDialog.tsx index a270bb7c58..9f07dc7085 100644 --- a/src/renderer/src/components/ConcatDialog.tsx +++ b/src/renderer/src/components/ConcatDialog.tsx @@ -13,11 +13,12 @@ import useFileFormatState from '../hooks/useFileFormatState'; import OutputFormatSelect from './OutputFormatSelect'; import useUserSettings from '../hooks/useUserSettings'; import { isMov } from '../util/streams'; -import { getOutFileExtension, getSuffixedFileName } from '../util'; +import { getOutDir, getOutFileExtension } from '../util'; import { FFprobeChapter, FFprobeFormat, FFprobeStream } from '../../../../ffprobe'; import Sheet from './Sheet'; import TextInput from './TextInput'; import Button from './Button'; +import { defaultMergedFileTemplate, generateMergedFileNames, maxFileNameLength } from '../util/outputNameTemplate'; const { basename } = window.require('path'); @@ -36,7 +37,7 @@ function ConcatDialog({ isShown, onHide, paths, onConcat, alwaysConcatMultipleFi isShown: boolean, onHide: () => void, paths: string[], onConcat: (a: { paths: string[], includeAllStreams: boolean, streams: FFprobeStream[], outFileName: string, fileFormat: string, clearBatchFilesAfterConcat: boolean }) => Promise, alwaysConcatMultipleFiles: boolean, setAlwaysConcatMultipleFiles: (a: boolean) => void, }) { const { t } = useTranslation(); - const { preserveMovData, setPreserveMovData, segmentsToChapters, setSegmentsToChapters, preserveMetadataOnMerge, setPreserveMetadataOnMerge } = useUserSettings(); + const { preserveMovData, setPreserveMovData, segmentsToChapters, setSegmentsToChapters, preserveMetadataOnMerge, setPreserveMetadataOnMerge, safeOutputFileName, customOutDir } = useUserSettings(); const [includeAllStreams, setIncludeAllStreams] = useState(false); const [fileMeta, setFileMeta] = useState<{ format: FFprobeFormat, streams: FFprobeStream[], chapters: FFprobeChapter[] }>(); @@ -80,16 +81,32 @@ function ConcatDialog({ isShown, onHide, paths, onConcat, alwaysConcatMultipleFi }, [firstPath, isShown, setDetectedFileFormat, setFileFormat]); useEffect(() => { - if (fileFormat == null || firstPath == null) { + if (fileFormat == null || firstPath == null || uniqueSuffix == null) { setOutFileName(undefined); return; } const ext = getOutFileExtension({ isCustomFormatSelected, outFormat: fileFormat, filePath: firstPath }); + const outputDir = getOutDir(customOutDir, firstPath); + setOutFileName((existingOutputName) => { - if (existingOutputName == null) return getSuffixedFileName(firstPath, `merged-${uniqueSuffix}${ext}`); - return existingOutputName.replace(/(\.[^.]*)?$/, ext); // make sure the last (optional) .* is replaced by .ext` + // here we only generate the file name the first time. Then the user can edit it manually as they please in the text input field. + // todo allow user to edit template instead of this "hack" + if (existingOutputName == null) { + (async () => { + const generated = await generateMergedFileNames({ template: defaultMergedFileTemplate, isCustomFormatSelected, fileFormat, filePath: firstPath, outputDir, safeOutputFileName, epochMs: uniqueSuffix }); + // todo show to user more errors? + const [fileName] = generated.fileNames; + invariant(fileName != null); + setOutFileName(fileName); + })(); + return existingOutputName; // async later (above) + } + + // in case the user has chosen a different output format: + // make sure the last (optional) .* is replaced by .ext` + return existingOutputName.replace(/(\.[^.]*)?$/, ext); }); - }, [fileFormat, firstPath, isCustomFormatSelected, uniqueSuffix]); + }, [customOutDir, fileFormat, firstPath, isCustomFormatSelected, safeOutputFileName, uniqueSuffix]); const allFilesMeta = useMemo(() => { if (paths.length === 0) return undefined; @@ -97,7 +114,8 @@ function ConcatDialog({ isShown, onHide, paths, onConcat, alwaysConcatMultipleFi return filtered.length === paths.length ? filtered : undefined; }, [allFilesMetaCache, paths]); - const isOutFileNameValid = outFileName != null && outFileName.length > 0; + const isOutFileNameTooLong = outFileName != null && outFileName.length > maxFileNameLength; + const isOutFileNameValid = outFileName != null && outFileName.length > 0 && !isOutFileNameTooLong; const problemsByFile = useMemo(() => { if (!allFilesMeta) return {}; @@ -209,9 +227,15 @@ function ConcatDialog({ isShown, onHide, paths, onConcat, alwaysConcatMultipleFi
{t('Output file name')}:
setOutFileName(e.target.value)} /> - +
+ {isOutFileNameTooLong && ( + + )} + {enableReadFileMeta && (!allFilesMeta || Object.values(problemsByFile).length > 0) && ( )} diff --git a/src/renderer/src/util/outputNameTemplate.ts b/src/renderer/src/util/outputNameTemplate.ts index beffa4cd29..bb18434ad1 100644 --- a/src/renderer/src/util/outputNameTemplate.ts +++ b/src/renderer/src/util/outputNameTemplate.ts @@ -15,6 +15,9 @@ export const segSuffixVariable = 'SEG_SUFFIX'; export const extVariable = 'EXT'; export const segTagsVariable = 'SEG_TAGS'; +// I don't remember why I set it to 200, but on Windows max length seems to be 256, on MacOS it seems to be 255. +export const maxFileNameLength = 200; + const { parse: parsePath, sep: pathSep, join: pathJoin, normalize: pathNormalize, basename }: PlatformPath = window.require('path'); @@ -104,7 +107,9 @@ function getTemplateProblems({ fileNames, filePath, outputDir, safeOutputFileNam // eslint-disable-next-line no-template-curly-in-string export const defaultOutSegTemplate = '${FILENAME}-${CUT_FROM}-${CUT_TO}${SEG_SUFFIX}${EXT}'; // eslint-disable-next-line no-template-curly-in-string -export const defaultMergedFileTemplate = '${FILENAME}-cut-merged-${EPOCH_MS}${EXT}'; +export const defaultCutMergedFileTemplate = '${FILENAME}-cut-merged-${EPOCH_MS}${EXT}'; +// eslint-disable-next-line no-template-curly-in-string +export const defaultMergedFileTemplate = '${FILENAME}-merged-${EPOCH_MS}${EXT}'; async function interpolateOutFileName(template: string, { epochMs, inputFileNameWithoutExt, ext, segSuffix, segNum, segNumPadded, segLabel, cutFrom, cutTo, tags }: { epochMs: number, @@ -151,7 +156,7 @@ function maybeTruncatePath(fileName: string, truncate: boolean) { return [ ...rest, // If sanitation is enabled, make sure filename (last seg of the path) is not too long - truncate ? lastSeg!.slice(0, 200) : lastSeg, + truncate ? lastSeg!.slice(0, maxFileNameLength) : lastSeg, ].join(pathSep); } @@ -223,17 +228,16 @@ export type GenerateOutFileNames = (a: { template: string }) => Promise<{ }, }>; -export async function generateMergedFileNames({ template: desiredTemplate, isCustomFormatSelected, fileFormat, filePath, outputDir, safeOutputFileName }: { +export async function generateMergedFileNames({ template: desiredTemplate, isCustomFormatSelected, fileFormat, filePath, outputDir, safeOutputFileName, epochMs = Date.now() }: { template: string, isCustomFormatSelected: boolean, fileFormat: string, filePath: string, outputDir: string, safeOutputFileName: boolean, + epochMs?: number, }) { async function generate(template: string) { - const epochMs = Date.now(); - const { name: inputFileNameWithoutExt } = parsePath(filePath); const fileName = await interpolateOutFileName(template, { @@ -249,7 +253,7 @@ export async function generateMergedFileNames({ template: desiredTemplate, isCus const problems = getTemplateProblems({ fileNames: [fileName], filePath, outputDir, safeOutputFileName }); if (problems.error != null) { - fileName = await generate(defaultMergedFileTemplate); + fileName = await generate(defaultCutMergedFileTemplate); } return { fileNames: [fileName], problems };