diff --git a/source/Program/function_change.c b/source/Program/function_change.c index 7cbef89c..d5fd48fb 100644 --- a/source/Program/function_change.c +++ b/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; diff --git a/source/Program/function_files.c b/source/Program/function_files.c index f9acf776..6b745d66 100644 --- a/source/Program/function_files.c +++ b/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) { diff --git a/source/Program/function_launch.h b/source/Program/function_launch.h index 75defa73..22323718 100644 --- a/source/Program/function_launch.h +++ b/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); diff --git a/source/Program/function_run.c b/source/Program/function_run.c index 960677b0..2582d64f 100644 --- a/source/Program/function_run.c +++ b/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))) diff --git a/source/Program/tests/test_comment_empty_arg.py b/source/Program/tests/test_comment_empty_arg.py new file mode 100644 index 00000000..81321e13 --- /dev/null +++ b/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() diff --git a/source/Program/tests/test_datestamp_date_args.py b/source/Program/tests/test_datestamp_date_args.py index 276ed17b..7d5e4ba4 100644 --- a/source/Program/tests/test_datestamp_date_args.py +++ b/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__": diff --git a/source/Program/tests/test_name_arg_brace_expansion.py b/source/Program/tests/test_name_arg_brace_expansion.py new file mode 100644 index 00000000..a47efd77 --- /dev/null +++ b/source/Program/tests/test_name_arg_brace_expansion.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Static checks for {fu}/{ou} NAME arguments on internal commands (issue #141). + +An internal command (e.g. Protect/Comment) invoked with NAME={fu} or NAME={ou} must operate +on the whole current selection and leave it selected. The mechanism: drop the NAME so the +entry list is built from the live selection, then rewind and flag every entry no-unselect +before the command runs. +""" + +from pathlib import Path +import unittest + + +ROOT = Path(__file__).resolve().parents[3] +PROGRAM = ROOT / "source" / "Program" + + +def read(name): + return (PROGRAM / name).read_text(encoding="latin-1") + + +class NameArgBraceExpansionTests(unittest.TestCase): + def test_standalone_helper_matches_codes_and_rejects_literals(self): + files_c = read("function_files.c") + self.assertIn("static BOOL function_arg_is_no_unselect_only(const char *arg)", files_c) + body = files_c[files_c.index("static BOOL function_arg_is_no_unselect_only") :] + body = body[: body.index("\n}\n")] + # Must recognise both no-unselect codes ({ou} = FUNC_ONE_FILE_NO_UNSELECT, + # {fu} = FUNC_ONE_PATH_NO_UNSELECT) + self.assertIn("FUNC_ONE_FILE_NO_UNSELECT", body) + self.assertIn("FUNC_ONE_PATH_NO_UNSELECT", body) + # Tolerate the code's own quote/suffix modifier bytes so plain {fu}/{ou} still match + self.assertIn("FUNC_NORMAL", body) + # Reject anything else (literal text, e.g. {fu}.info) via a default that returns 0 + self.assertIn("default:", body) + self.assertIn("return 0;", body) + + def test_gate_is_scoped_to_standalone_no_unselect_internal_commands(self): + files_c = read("function_files.c") + self.assertIn("short function_brace_name_arg(InstructionParsed *instruction)", files_c) + gate = files_c[files_c.index("short function_brace_name_arg") :] + gate = gate[: gate.index("\n}\n")] + # Internal commands only, excluding external functions + self.assertIn("INST_COMMAND", gate) + self.assertIn("FUNCF_EXTERNAL_FUNCTION", gate) + # The file-supplying argument + self.assertIn("FUNCKEY_FILE", gate) + self.assertIn("FUNCKEY_FILENO", gate) + # Scoped to a STANDALONE no-unselect code, not any arg that merely contains one + self.assertIn("function_arg_is_no_unselect_only", gate) + # The old generic control-byte helper must be gone + self.assertNotIn("function_arg_has_control_bytes", files_c) + + def test_drop_clears_the_name_argument(self): + files_c = read("function_files.c") + self.assertIn("void function_drop_brace_name_arg(InstructionParsed *instruction)", files_c) + body = files_c[files_c.index("void function_drop_brace_name_arg") :] + body = body[: body.index("\n}\n")] + # Gate on the predicate, then null the NAME so build_list gathers the selection + self.assertIn("function_brace_name_arg(instruction)", body) + self.assertIn("instruction->funcargs->FA_Arguments[num] = 0;", body) + # The throwaway gather/expand/discard approach must be gone + self.assertNotIn("NewList(&handle->entry_list);", body) + self.assertNotIn("function_parse_arguments", body) + + def test_prototypes_present(self): + launch_h = read("function_launch.h") + self.assertIn("void function_drop_brace_name_arg(InstructionParsed *);", launch_h) + self.assertIn("short function_brace_name_arg(InstructionParsed *);", launch_h) + # The no-unselect helper is file-local (static), so not exported + self.assertNotIn("function_arg_is_no_unselect_only", launch_h) + self.assertNotIn("function_arg_has_no_unselect", launch_h) + # The old resolver prototype must be gone + self.assertNotIn("function_resolve_name_args", launch_h) + + def test_run_drops_name_before_building_list(self): + run_c = read("function_run.c") + idx_drop = run_c.index("function_drop_brace_name_arg(instruction)") + idx_build = run_c.index("function_build_list(handle, &path, instruction)") + self.assertLess(idx_drop, idx_build) + + def test_run_rewinds_and_keeps_whole_selection_selected(self): + run_c = read("function_run.c") + # The rewind/flag lives between argument parsing and the command call + block = run_c[run_c.index("function_parse_arguments(handle, instruction);") :] + block = block[: block.index("function_internal_command")] + self.assertIn("function_brace_name_arg(instruction) >= 0", block) + # Rewind current_entry to the head of the selection + self.assertIn("handle->current_entry = (FunctionEntry *)handle->entry_list.lh_Head;", block) + # Walk the whole list marking every entry no-unselect + self.assertIn("bent->node.mln_Succ", block) + self.assertIn("bent->flags |= FUNCENTF_NO_UNSELECT;", block) + + def test_change_reselects_no_unselect_entries(self): + # Changing protection/comment/date replaces the lister entry, which clears its selected + # state; a no-unselect entry must be reselected via FCF_SELECT on the filechange. + change_c = read("function_change.c") + block = change_c[change_c.index("function_filechange_addfile") :] + block = block[: block.index("FUNCENTF_REMOVE")] + self.assertIn("entry->flags & FUNCENTF_NO_UNSELECT", block) + self.assertIn("FCF_SELECT", block) + + +if __name__ == "__main__": + unittest.main()