Fix internal command {} arguments: Comment "" and {fu}/{ou} NAME (#141)

source/Program/function_change.c

@@ -27,7 +27,7 @@ For more information on Directory Opus for Windows please see:
 static int function_change_get_comment(FunctionHandle *handle, char *file, char *buffer);
 static int function_change_get_protect(FunctionHandle *handle, char *file, ULONG old_prot, unsigned char *masks);
 static int function_change_get_date(FunctionHandle *handle, char *file, struct DateStamp *date);
-static BOOL function_change_date_arg_empty(char *date);
+static BOOL function_change_arg_empty(char *arg);
 
 #ifndef __amigaos3__
 	#pragma pack(2)
@@ -80,8 +80,14 @@ DOPUS_FUNC(function_change)
 			// Comment supplied?
 			if (instruction->funcargs->FA_Arguments[1])
 			{
-				// Copy comment
-				stccpy(data->comment, (char *)instruction->funcargs->FA_Arguments[1], 80);
+				// An empty (or quoted-empty) comment clears the filenote
+				if (function_change_arg_empty((char *)instruction->funcargs->FA_Arguments[1]))
+					data->comment[0] = 0;
+
+				// Otherwise copy the supplied comment
+				else
+					stccpy(data->comment, (char *)instruction->funcargs->FA_Arguments[1], 80);
+
 				handle->inst_flags |= INSTF_NO_ASK;
 			}
 
@@ -140,7 +146,7 @@ DOPUS_FUNC(function_change)
 			if (instruction->funcargs->FA_Arguments[2])
 			{
 				// Null date?
-				if (function_change_date_arg_empty((char *)instruction->funcargs->FA_Arguments[2]))
+				if (function_change_arg_empty((char *)instruction->funcargs->FA_Arguments[2]))
 					DateStamp(&data->date);
 
 				// Otherwise
@@ -439,6 +445,7 @@ DOPUS_FUNC(function_change)
 				if (change && path->lister)
 				{
 					struct FileInfoBlock fib;
+					FileChange *filechange;
 
 					// Fill out dummy fileinfoblock
 					strcpy(fib.fib_FileName, entry->name);
@@ -452,12 +459,19 @@ DOPUS_FUNC(function_change)
 					fib.fib_Protection = protection;
 
 					// Add new file
-					if (function_filechange_addfile(handle,
-													path->path,
-													&fib,
-													(NetworkInfo *)GetTagData(DE_NetworkInfo, 0, entry->entry->de_Tags),
-													0))
+					if ((filechange = function_filechange_addfile(
+							 handle,
+							 path->path,
+							 &fib,
+							 (NetworkInfo *)GetTagData(DE_NetworkInfo, 0, entry->entry->de_Tags),
+							 0)))
 					{
+						// Changing an entry replaces it in the lister, which clears its selected
+						// state. If the {fu}/{ou} no-unselect intent applies, reselect the new entry
+						// so the selection is preserved.
+						if (entry->flags & FUNCENTF_NO_UNSELECT)
+							filechange->node.ln_Pri |= FCF_SELECT;
+
 						// Mark old entry for removal
 						entry->flags |= FUNCENTF_REMOVE;
 					}
@@ -761,35 +775,35 @@ static int function_change_get_protect(FunctionHandle *handle, char *file, ULONG
 	return 1;
 }
 
-// Check for an empty date argument, including DATE="" if quotes survive parsing
-static BOOL function_change_date_arg_empty(char *date)
+// Check for an empty argument, including ARG="" if quotes survive parsing
+static BOOL function_change_arg_empty(char *arg)
 {
 	char *end;
 
-	if (!date)
+	if (!arg)
 		return 0;
 
-	while (*date && isspace(*date))
-		++date;
+	while (*arg && isspace(*arg))
+		++arg;
 
-	end = date + strlen(date);
-	while (end > date && isspace(*(end - 1)))
+	end = arg + strlen(arg);
+	while (end > arg && isspace(*(end - 1)))
 		--end;
 
-	if (end == date)
+	if (end == arg)
 		return 1;
 
-	if ((*date == '"' || *date == '\'') && end > date && *(end - 1) == *date)
+	if ((*arg == '"' || *arg == '\'') && end > arg && *(end - 1) == *arg)
 	{
-		++date;
+		++arg;
 		--end;
 
-		while (date < end && isspace(*date))
-			++date;
-		while (end > date && isspace(*(end - 1)))
+		while (arg < end && isspace(*arg))
+			++arg;
+		while (end > arg && isspace(*(end - 1)))
 			--end;
 
-		return (end == date);
+		return (end == arg);
 	}
 
 	return 0;

source/Program/function_files.c

@@ -68,6 +68,91 @@ LONG MatchNext64Plus(struct AnchorPath *panchor, ULONG blocksize)
 static int function_check_filter(FunctionHandle *handle);
 static FunctionEntry *function_new_entry_on_list(FunctionHandle *handle, struct List *list, char *name, BOOL icon);
 
+// Is an argument a standalone no-unselect selection reference, i.e. exactly {fu} or {ou}
+// (the no-unselect code plus its own quote/suffix modifier bytes), with no literal text?
+// An embedded code such as {fu}.info or old-{ou} carries literal characters and names
+// specific files, so it must NOT be treated as "operate on the whole selection".
+static BOOL function_arg_is_no_unselect_only(const char *arg)
+{
+	BOOL found = 0;
+
+	if (!arg)
+		return 0;
+
+	for (; *arg; ++arg)
+	{
+		switch ((unsigned char)*arg)
+		{
+		// The no-unselect file/path codes themselves
+		case FUNC_ONE_FILE_NO_UNSELECT:
+		case FUNC_ONE_PATH_NO_UNSELECT:
+			found = 1;
+			break;
+
+		// Quote/suffix modifier bytes the parser appends to a single file/path code
+		case FUNC_QUOTES:
+		case FUNC_NO_QUOTES:
+		case FUNC_NORMAL:
+		case FUNC_STRIP_SUFFIX:
+			break;
+
+		// Literal text or any other code: the {} reference is part of a larger filename
+		default:
+			return 0;
+		}
+	}
+
+	return found;
+}
+
+// If an internal command supplies its file via a NAME/file template argument that is a
+// standalone no-unselect {} code ({fu}/{ou}), return that argument's index; otherwise return
+// -1. FA_ArgArray[] keeps the raw (control-byte) argument even after expansion, so this stays
+// valid in both phases.
+short function_brace_name_arg(InstructionParsed *instruction)
+{
+	char *key;
+	short num;
+
+	// Internal command with a template and parsed arguments only
+	if (!instruction || instruction->type != INST_COMMAND || !instruction->command ||
+		(instruction->command->flags & FUNCF_EXTERNAL_FUNCTION) || !instruction->command->template_key ||
+		!instruction->funcargs)
+		return -1;
+
+	// Find the file-supplying argument
+	if (!(key = strchr(instruction->command->template_key, FUNCKEY_FILE)) &&
+		!(key = strchr(instruction->command->template_key, FUNCKEY_FILENO)))
+		return -1;
+
+	num = atoi(key + 1);
+
+	// Only when that argument is a standalone no-unselect code ({fu}/{ou}), not one embedded
+	// in a larger filename (e.g. {fu}.info), which names specific files
+	if (!instruction->funcargs->FA_ArgArray[num] ||
+		!function_arg_is_no_unselect_only((char *)instruction->funcargs->FA_ArgArray[num]))
+		return -1;
+
+	return num;
+}
+
+// An internal command whose NAME/file argument is a no-unselect code ({fu}/{ou}) means
+// "operate on the current selection and keep it selected". Drop the argument so
+// function_build_list() gathers the live selection (with its real lister entries) instead of
+// treating the expanded control-byte string as a filename/path to look up: the latter loses
+// the lister-entry link for {fu} (which expands to a full path) and only ever yields a single
+// file. The selection is rewound and kept selected later, in function_run_instruction(), once
+// argument expansion has finished consuming entries.
+void function_drop_brace_name_arg(InstructionParsed *instruction)
+{
+	short num;
+
+	if ((num = function_brace_name_arg(instruction)) < 0)
+		return;
+
+	instruction->funcargs->FA_Arguments[num] = 0;
+}
+
 // Build entry list for a function
 int function_build_list(FunctionHandle *handle, PathNode **path, InstructionParsed *instruction)
 {

source/Program/function_launch.h

@@ -473,6 +473,8 @@ BOOL function_lock_paths(FunctionHandle *handle, PathList *, int);
 void function_unlock_paths(FunctionHandle *handle, PathList *, int);
 
 int function_build_list(FunctionHandle *function, PathNode **, InstructionParsed *);
+void function_drop_brace_name_arg(InstructionParsed *);
+short function_brace_name_arg(InstructionParsed *);
 int function_build_dest_list(FunctionHandle *function, PathNode **, InstructionParsed *);
 FunctionEntry *function_new_entry(FunctionHandle *, char *, BOOL);
 FunctionEntry *function_current_entry(FunctionHandle *handle);

source/Program/function_run.c

@@ -166,6 +166,10 @@ short function_run(FunctionHandle *handle)
 				// Are we out of files?
 				if (!function_current_entry(handle))
 				{
+					// A {fu}/{ou} NAME argument on an internal command means "use the selection":
+					// drop it so the entry list below is built from the live selection
+					function_drop_brace_name_arg(instruction);
+
 					// Build new entry list
 					if ((function_build_list(handle, &path, instruction)) == -1)
 					{
@@ -317,6 +321,21 @@ static int function_run_instruction(FunctionHandle *handle, InstructionParsed *i
 					// Check arguments
 					function_parse_arguments(handle, instruction);
 
+					// For an internal command with a {fu}/{ou} NAME argument, the entry list holds
+					// the whole selection (see function_drop_brace_name_arg). Expanding the {fu}/{ou}
+					// code while building the arguments above consumed one or more of those entries;
+					// rewind so the command processes the entire selection, and mark every entry
+					// no-unselect so it stays selected afterwards.
+					if (function_brace_name_arg(instruction) >= 0)
+					{
+						FunctionEntry *bent;
+
+						handle->current_entry = (FunctionEntry *)handle->entry_list.lh_Head;
+						for (bent = (FunctionEntry *)handle->entry_list.lh_Head; bent->node.mln_Succ;
+							 bent = (FunctionEntry *)bent->node.mln_Succ)
+							bent->flags |= FUNCENTF_NO_UNSELECT;
+					}
+
 					// If we needs files, check there are some
 					if (!(instruction->flags & FUNCF_NEED_ENTRIES) || instruction->flags & FUNCF_WANT_ENTRIES ||
 						(function_current_entry(handle)))

source/Program/tests/test_comment_empty_arg.py

@@ -0,0 +1,38 @@
+#!/usr/bin/env python3
+"""Regression checks for empty Comment argument handling (issue #141)."""
+
+from pathlib import Path
+import unittest
+
+
+ROOT = Path(__file__).resolve().parents[3]
+FUNCTION_CHANGE_C = ROOT / "source" / "Program" / "function_change.c"
+
+
+def read_source():
+    return FUNCTION_CHANGE_C.read_text(encoding="latin-1")
+
+
+class CommentEmptyArgumentTests(unittest.TestCase):
+    def test_shared_empty_arg_predicate_exists(self):
+        source = read_source()
+        self.assertIn("static BOOL function_change_arg_empty(char *arg)", source)
+        # Old date-specific name must be fully gone
+        self.assertNotIn("function_change_date_arg_empty", source)
+
+    def test_empty_comment_clears_filenote_without_prompting(self):
+        source = read_source()
+        block = source[
+            source.index("if (command->function == FUNC_COMMENT)")
+            : source.index("else if (command->function == FUNC_PROTECT)")
+        ]
+        self.assertIn(
+            "function_change_arg_empty((char *)instruction->funcargs->FA_Arguments[1])",
+            block,
+        )
+        self.assertIn("data->comment[0] = 0;", block)
+        self.assertIn("INSTF_NO_ASK", block)
+
+
+if __name__ == "__main__":
+    unittest.main()

source/Program/tests/test_datestamp_date_args.py

@@ -19,26 +19,26 @@ def test_empty_date_argument_uses_current_datestamp_before_parsing(self):
         source = read_source()
         setup = source[source.index("// Arguments supplied?") : source.index("// Any directories selected?")]
 
-        self.assertIn("function_change_date_arg_empty", source)
-        self.assertIn("if (function_change_date_arg_empty((char *)instruction->funcargs->FA_Arguments[2]))", setup)
+        self.assertIn("function_change_arg_empty", source)
+        self.assertIn("if (function_change_arg_empty((char *)instruction->funcargs->FA_Arguments[2]))", setup)
         self.assertRegex(
             setup,
             re.compile(
-                r"function_change_date_arg_empty\(\(char \*\)instruction->funcargs->FA_Arguments\[2\]\)\)\s*"
+                r"function_change_arg_empty\(\(char \*\)instruction->funcargs->FA_Arguments\[2\]\)\)\s*"
                 r"DateStamp\(&data->date\);.*?"
                 r"else\s*\{.*?ParseDateStrings",
                 re.S,
             ),
         )
 
-    def test_empty_date_argument_helper_handles_quoted_empty_strings(self):
+    def test_empty_arg_helper_handles_quoted_empty_strings(self):
         source = read_source()
-        helper = source[source.index("static BOOL function_change_date_arg_empty") :]
+        helper = source[source.index("static BOOL function_change_arg_empty") :]
 
-        self.assertIn("*date == '\"'", helper)
-        self.assertIn("*date == '\\''", helper)
-        self.assertRegex(helper, r"while \(\*date && isspace\(\*date\)\)")
-        self.assertRegex(helper, r"return \(end == date\);")
+        self.assertIn("*arg == '\"'", helper)
+        self.assertIn("*arg == '\\''", helper)
+        self.assertRegex(helper, r"while \(\*arg && isspace\(\*arg\)\)")
+        self.assertRegex(helper, r"return \(end == arg\);")
 
 
 if __name__ == "__main__":