The plain text export named its files after the script ID instead of …

README.md

@@ -28,6 +28,8 @@ Executes Javascript, Typescript Scripts.
 
 ## Changelog
 ### **WORK IN PROGRESS**
+* (@GermanBluefox) The plain text export named its files after the script ID instead of the script name, so every dot of a name came out as an underscore - `HK-Balkontuer_v0.1` was exported as `HK-Balkontuer_v0_1.js`, and importing it back renamed the script to that. The files are now named after the script (#2364)
+* (@GermanBluefox) Importing a plain text export treated a dot inside a file name as a folder level, so `PW-TV-Control_v0.6.js` created a folder `PW-TV-Control_v0` containing a script named `6`. Only the directories of the ZIP are folders now (#2364)
 * (@GermanBluefox) The folder icons in the script tree were drawn at less than half the size of the script icons next to them: they spaced themselves with a padding, and since `CssBaseline` sets `box-sizing: border-box` that padding was subtracted from their 20px instead of being added to them. They use a margin now, like the script icons always did (#2360)
 * (@GermanBluefox) The log below the editor could not be resized while a script was open: the editor area guessed its height from the height the tabs and the toolbar were expected to have, hung over the bottom edge of its pane and covered the 8px splitter with the horizontal scrollbar of the editor, which swallowed the mouse click. The three parts now share the height as a flex column (#2351)
 * (@GermanBluefox) The script list cut off long names, although there was still free space next to them: the space for the buttons at the end of a row was a fixed 185px, which is more than the three buttons occupy, and it did not account for the icon column

src-editor/src/App.tsx

@@ -39,6 +39,7 @@ import ruLang from './i18n/ru.json';
 import ukLang from './i18n/uk.json';
 import zhCnLang from './i18n/zh-cn.json';
 import JSZip from 'jszip';
+import { getScriptName, nameToFileName, scriptIdToZipFolder, zipPathToScript } from '@/scriptNames';
 import type { ScriptType } from '@/types';
 import PasswordDialog from '@/Dialogs/Password';
 
@@ -953,6 +954,7 @@ export default class App extends GenericApp<AppProps, AppState> {
         } else {
             // Export as ZIP with same structure of scripts but in plain text
             const zip = new JSZip();
+            const usedPaths = new Set<string>();
             for (const [id, obj] of Object.entries(this.scripts)) {
                 if (obj.type === 'script') {
                     const scriptObj = obj;
@@ -966,9 +968,22 @@ export default class App extends GenericApp<AppProps, AppState> {
                                 : 'js';
                     let text = `/******* (ext=${ext}/engine=${scriptObj.common.engine}/debug=${scriptObj.common.debug}/verbose=${scriptObj.common.verbose}/enabled=${scriptObj.common.enabled}) *******/\n`;
                     text += scriptObj.common.source || '';
-                    // Convert dots in the path to slashes to create folder structure, e.g. common.myFolder.myScript → common/myFolder/myScript.js
-                    const filePath = `${id.substring('script.js.'.length).replace(/\./g, '/')}.${ext}`;
-                    zip.file(filePath, text);
+
+                    // The dots of the ID are the folder structure, but the file is named after the
+                    // script and not after the last level of the ID: an ID cannot hold a dot, so a
+                    // script called "v0.1" is stored there as "v0_1" - naming the file after it
+                    // silently renamed the script on the way out (#2364)
+                    const folder = scriptIdToZipFolder(id);
+                    let fileName = nameToFileName(getScriptName(id, scriptObj, I18n.getLanguage()));
+
+                    // Two scripts of one folder may carry the same name, their IDs cannot
+                    const key = `${folder}/${fileName}.${ext}`.toLowerCase();
+                    if (usedPaths.has(key)) {
+                        fileName = `${fileName}_${id.split('.').pop()}`;
+                    }
+                    usedPaths.add(key);
+
+                    zip.file(`${folder ? `${folder}/` : ''}${fileName}.${ext}`, text);
                 }
             }
             void zip.generateAsync({ type: 'blob' }).then(blob => {
@@ -1047,10 +1062,8 @@ export default class App extends GenericApp<AppProps, AppState> {
                                     }
                                 }
 
-                                // Convert file path back to script ID: common/myFolder/myScript.js → script.js.common.myFolder.myScript
-                                const scriptPath = relativePath.replace(/\.\w+$/, '').replace(/\//g, '.');
-                                const id = `script.js.${scriptPath}`;
-                                const name = scriptPath.split('.').pop() || scriptPath;
+                                // common/myFolder/myScript.js → script.js.common.myFolder.myScript
+                                const { id, name, parts: pathParts } = zipPathToScript(relativePath);
 
                                 // Ensure parent folders exist
                                 const parts = id.split('.');
@@ -1062,7 +1075,9 @@ export default class App extends GenericApp<AppProps, AppState> {
                                                 _id: folderId,
                                                 type: 'channel',
                                                 common: {
-                                                    name: parts[i - 1],
+                                                    // the unsanitized path part - a folder may carry
+                                                    // a dot in its name too
+                                                    name: pathParts[i - 3] || parts[i - 1],
                                                     expert: true,
                                                 },
                                                 native: {},

src-editor/src/Dialogs/New.tsx

@@ -17,6 +17,8 @@ import {
 import { Check as IconOk, Cancel as IconCancel, Clear as ClearIcon } from '@mui/icons-material';
 
 import { I18n } from '@iobroker/gui-components';
+
+import { nameToIdPart } from '@/scriptNames';
 import type { ScriptType } from '@/types';
 
 interface DialogNewProps {
@@ -63,12 +65,8 @@ class DialogNew extends React.Component<DialogNewProps, DialogNewState> {
     }
 
     getId(name?: string): string {
-        name = name || this.state.name || '';
-        name = name
-            .replace(/[\\/\][.*,;'"`<>?\s]/g, '_')
-            .trim()
-            .replace(/\.$/, '_');
-        return `${this.state ? this.state.parent : this.props.parent}.${name}`;
+        const idPart = nameToIdPart(name || this.state.name || '');
+        return `${this.state ? this.state.parent : this.props.parent}.${idPart}`;
     }
 
     handleOk = (): void => {

src-editor/src/Dialogs/Rename.tsx

@@ -17,6 +17,8 @@ import { Cancel as IconCancel, Check as IconOk } from '@mui/icons-material';
 
 import { I18n } from '@iobroker/gui-components';
 
+import { nameToIdPart } from '@/scriptNames';
+
 interface DialogRenameProps {
     onClose: () => void;
     onRename: (oldId: string, newId: string, newName?: string, newInstance?: number) => void;
@@ -66,11 +68,7 @@ class DialogRename extends React.Component<DialogRenameProps, DialogRenameState>
     }
 
     getId(name: string): string {
-        name = (name || '')
-            .replace(/[\\/\][.*,;'"`<>?\s]/g, '_')
-            .trim()
-            .replace(/\.$/, '_');
-        return `${this.state.prefix}.${name}`;
+        return `${this.state.prefix}.${nameToIdPart(name)}`;
     }
 
     handleCancel = (): void => {

src-editor/src/scriptNames.ts

@@ -0,0 +1,106 @@
+/**
+ * Turning script names into IDs and file names, and back.
+ *
+ * An ioBroker ID separates its levels with dots, so a single level cannot contain one: the script
+ * `HK-Balkontuer_v0.1` has to live under the ID `script.js.common.HK-Balkontuer_v0_1`. That is why
+ * the name is kept in `common.name` and only the ID is sanitized - the tree shows the name and
+ * nothing of it is lost.
+ *
+ * What did get lost was everything built from the ID instead of the name: the plain text export
+ * named its files after the ID, so the dots of a name came out as underscores, and the import read
+ * the name back out of the file path, where a dot then opened a folder of its own (#2364).
+ *
+ * The module has no imports on purpose - it is the one place that knows these rules, and a unit
+ * test can run it as it is.
+ */
+
+/** Everything a file name must not contain on Windows, macOS or Linux - the dot is not among them */
+// eslint-disable-next-line no-control-regex
+const FORBIDDEN_IN_FILE_NAME = /[\\/:*?"<>|\u0000-\u001F]/g;
+
+/** Where every script ID starts */
+const ROOT = 'script.js.';
+
+/**
+ * The name of a script or folder as the user sees it in the tree.
+ *
+ * @param id ID of the object, used when it carries no name
+ * @param obj The object
+ * @param lang Language to pick from a translated name
+ */
+export function getScriptName(id: string, obj?: ioBroker.Object | null, lang?: ioBroker.Languages): string {
+    const name = obj?.common?.name;
+
+    if (name) {
+        if (typeof name === 'object') {
+            return (name[lang || 'en'] || name.en || id.replace(/^script\.js\./, '')).toString();
+        }
+        return name.toString();
+    }
+
+    return id.replace(/^script\.js\./, '');
+}
+
+/**
+ * Makes one level of an ID out of a name.
+ *
+ * The dot is part of the replaced set: it would otherwise open a new level, and the script would
+ * silently end up inside a folder named after the first half of its name.
+ *
+ * @param name The name the user entered
+ */
+export function nameToIdPart(name: string): string {
+    return (name || '')
+        .replace(/[\\/\][.*,;'"`<>?\s]/g, '_')
+        .trim()
+        .replace(/\.$/, '_');
+}
+
+/**
+ * Makes a file name out of a script name.
+ *
+ * Unlike `nameToIdPart` the dot survives - it is legal in a file name, and keeping it is the whole
+ * point: the export is supposed to write `HK-Balkontuer_v0.1.js` and not `HK-Balkontuer_v0_1.js`.
+ *
+ * @param name The name of the script
+ */
+export function nameToFileName(name: string): string {
+    const fileName = (name || '')
+        .replace(FORBIDDEN_IN_FILE_NAME, '_')
+        // Windows drops a trailing dot or space without a word
+        .replace(/[. ]+$/, '')
+        // a leading dot would make the file hidden on Linux and macOS
+        .replace(/^\./, '_');
+
+    return fileName || '_';
+}
+
+/**
+ * The directory a script belongs into inside the plain text export, without a trailing slash.
+ *
+ * Only the folders come from the ID - they are what its dots really mean. The file itself is named
+ * after the script, see `nameToFileName`.
+ *
+ * @param id ID of the script
+ */
+export function scriptIdToZipFolder(id: string): string {
+    return (id.startsWith(ROOT) ? id.substring(ROOT.length) : id).split('.').slice(0, -1).join('/');
+}
+
+/**
+ * Reads a path inside the plain text export back.
+ *
+ * Only the slashes are levels. A dot inside a file name belongs to the name, so it is sanitized for
+ * the ID exactly the way the rename dialog does it, while the returned name keeps it.
+ *
+ * @param relativePath Path of the file inside the ZIP, e.g. `common/Heizung/HK-Balkontuer_v0.1.js`
+ */
+export function zipPathToScript(relativePath: string): { id: string; name: string; parts: string[] } {
+    const parts = relativePath.replace(/\.\w+$/, '').split('/');
+
+    return {
+        parts,
+        name: parts[parts.length - 1] || relativePath,
+        id: `${ROOT}${parts.map(part => nameToIdPart(part)).join('.')}`,
+    };
+}

test/testScriptNames.js

@@ -0,0 +1,166 @@
+'use strict';
+
+/**
+ * Names of scripts on their way into an ID, into the plain text export and back (#2364).
+ *
+ * An ioBroker ID cannot hold a dot inside one level, so `HK-Balkontuer_v0.1` has to become
+ * `script.js.common.HK-Balkontuer_v0_1`. The name itself survives in `common.name` - but everything
+ * that was built from the ID instead of the name lost it again: the export named its files after the
+ * ID, and the import read the name back out of the file path, where a dot opened a folder.
+ *
+ * Run: mocha test/testScriptNames.js --exit
+ */
+const assert = require('node:assert').strict;
+const { join } = require('node:path');
+const { buildSync } = require('esbuild');
+
+const SOURCE = join(__dirname, '..', 'src-editor', 'src', 'scriptNames.ts');
+
+/** The module has no imports, so esbuild only has to strip the types */
+function loadScriptNames() {
+    const { text } = buildSync({
+        entryPoints: [SOURCE],
+        bundle: true,
+        format: 'cjs',
+        platform: 'node',
+        write: false,
+        logLevel: 'silent',
+    }).outputFiles[0];
+
+    const module = { exports: {} };
+    // eslint-disable-next-line no-new-func
+    new Function('module', 'exports', text)(module, module.exports);
+    return module.exports;
+}
+
+describe('Script names', function () {
+    this.timeout(30000);
+
+    let names;
+
+    before(() => {
+        names = loadScriptNames();
+    });
+
+    describe('nameToIdPart', () => {
+        it('replaces the dot, which would otherwise open a folder', () => {
+            assert.equal(names.nameToIdPart('HK-Balkontuer_v0.1'), 'HK-Balkontuer_v0_1');
+            assert.equal(names.nameToIdPart('PWSW-Master-Slave_v0.10_Buero'), 'PWSW-Master-Slave_v0_10_Buero');
+        });
+
+        it('replaces everything else an ID must not contain', () => {
+            assert.equal(names.nameToIdPart('a/b\\c[d]e*f,g;h'), 'a_b_c_d_e_f_g_h');
+            assert.equal(names.nameToIdPart('with spaces'), 'with_spaces');
+            assert.equal(names.nameToIdPart('trailing.'), 'trailing_');
+        });
+
+        it('leaves a harmless name alone', () => {
+            assert.equal(names.nameToIdPart('PW-TV-Control_v0_6_VH_OG1'), 'PW-TV-Control_v0_6_VH_OG1');
+            assert.equal(names.nameToIdPart('Heizung-Bad'), 'Heizung-Bad');
+        });
+    });
+
+    describe('nameToFileName', () => {
+        it('keeps the dot - that is the whole point', () => {
+            assert.equal(names.nameToFileName('HK-Balkontuer_v0.1'), 'HK-Balkontuer_v0.1');
+            assert.equal(names.nameToFileName('PW-TV-Control_v0.6_VH_OG1'), 'PW-TV-Control_v0.6_VH_OG1');
+        });
+
+        it('keeps spaces and umlauts', () => {
+            assert.equal(names.nameToFileName('Rollladen Büro'), 'Rollladen Büro');
+        });
+
+        it('replaces what a file name must not contain', () => {
+            assert.equal(names.nameToFileName('a/b'), 'a_b');
+            assert.equal(names.nameToFileName('a\\b:c*d?e"f<g>h|i'), 'a_b_c_d_e_f_g_h_i');
+        });
+
+        it('avoids names the file system would mangle', () => {
+            // Windows silently drops a trailing dot or space
+            assert.equal(names.nameToFileName('Skript.'), 'Skript');
+            assert.equal(names.nameToFileName('Skript '), 'Skript');
+            // a leading dot would hide the file
+            assert.equal(names.nameToFileName('.hidden'), '_hidden');
+            // and something has to be left over
+            assert.equal(names.nameToFileName('...'), '_');
+            assert.equal(names.nameToFileName(''), '_');
+        });
+    });
+
+    describe('the plain text export', () => {
+        it('takes the folders from the ID and the file name from the script name', () => {
+            const id = 'script.js.common.Heizung.HK-Balkontuer_v0_1';
+
+            assert.equal(names.scriptIdToZipFolder(id), 'common/Heizung');
+            assert.equal(names.nameToFileName('HK-Balkontuer_v0.1'), 'HK-Balkontuer_v0.1');
+        });
+
+        it('puts a script without a folder into the root of the ZIP', () => {
+            assert.equal(names.scriptIdToZipFolder('script.js.Skript_1'), '');
+        });
+
+        it('reads a path back without inventing a folder for the dot', () => {
+            const script = names.zipPathToScript('common/Heizung/HK-Balkontuer_v0.1.js');
+
+            assert.equal(script.id, 'script.js.common.Heizung.HK-Balkontuer_v0_1');
+            assert.equal(script.name, 'HK-Balkontuer_v0.1');
+            assert.deepEqual(script.parts, ['common', 'Heizung', 'HK-Balkontuer_v0.1']);
+        });
+
+        it('reads every extension the export writes', () => {
+            for (const ext of ['js', 'ts', 'blockly', 'rules']) {
+                const script = names.zipPathToScript(`common/Skript_1.${ext}`);
+                assert.equal(script.id, 'script.js.common.Skript_1', ext);
+                assert.equal(script.name, 'Skript_1', ext);
+            }
+        });
+
+        it('survives the round trip with the names from the report', () => {
+            const scripts = [
+                { id: 'script.js.common.PW-TV-Control_v0_6_VH_OG1', name: 'PW-TV-Control_v0.6_VH_OG1' },
+                { id: 'script.js.common.PWSW-Master-Slave_v0_10_Büro', name: 'PWSW-Master-Slave_v0.10_Büro' },
+                { id: 'script.js.common.Heizung.HK-Balkontuer_v0_1', name: 'HK-Balkontuer_v0.1' },
+                { id: 'script.js.Skript_1', name: 'Skript 1' },
+            ];
+
+            for (const script of scripts) {
+                const folder = names.scriptIdToZipFolder(script.id);
+                const path = `${folder ? `${folder}/` : ''}${names.nameToFileName(script.name)}.js`;
+                const back = names.zipPathToScript(path);
+
+                assert.equal(back.name, script.name, `name of ${path}`);
+                assert.equal(back.id, script.id, `id of ${path}`);
+            }
+        });
+
+        it('never lets a dot in a name become a folder', () => {
+            // this is what produced the reported "6.json": the version number ended up as a script
+            // of its own inside a folder named after the first half
+            const script = names.zipPathToScript('common/PW-TV-Control_v0.6.js');
+
+            assert.equal(script.parts.length, 2, 'the path has one folder and one file');
+            assert.equal(script.id.split('.').length, 4, `unexpected levels in ${script.id}`);
+            assert.equal(script.id, 'script.js.common.PW-TV-Control_v0_6');
+        });
+    });
+
+    describe('getScriptName', () => {
+        it('takes a plain name', () => {
+            assert.equal(names.getScriptName('script.js.common.a', { common: { name: 'My script' } }), 'My script');
+        });
+
+        it('takes the requested language of a translated name', () => {
+            const obj = { common: { name: { en: 'Heating', de: 'Heizung' } } };
+
+            assert.equal(names.getScriptName('script.js.common.a', obj, 'de'), 'Heizung');
+            assert.equal(names.getScriptName('script.js.common.a', obj, 'en'), 'Heating');
+            // falls back to English for a language the name does not have
+            assert.equal(names.getScriptName('script.js.common.a', obj, 'fr'), 'Heating');
+        });
+
+        it('falls back to the ID without its root', () => {
+            assert.equal(names.getScriptName('script.js.common.a', { common: {} }), 'common.a');
+            assert.equal(names.getScriptName('script.js.common.a', null), 'common.a');
+        });
+    });
+});