Skip to content

Commit 3982a8c

Browse files
Fix format on save ignored directories path matching logic (#325)
* fix(plugin): resolve ignored directories never matching due to trailing slash Fixes a bug introduced in #191 Directory paths with a trailing "/" (e.g. "addons/") would produce a trailing empty string when split on "/", which could never match a real path segment. This silently broke the ignore rule for any entry with a trailing slash. This fix trims the trailing "/" before splitting. * fix(plugin): resolve out-of-bounds access when ignored directory is deeper than script path Fixes a bug introduced in #191 The matching loop indexed script_path_parts[i] using the length of directory_parts, without checking that directory_parts wasn't longer. If an ignored directory's path has more segments than the script's path, and every one of those extra segments follows a full match of the script's path, the loop runs past the end of script_path_parts. Example: script at "res://addons/foo.gd" (script_path_parts = ["addons", "foo.gd"]) checked against ignored directory "addons/foo.gd/bar" (directory_parts = ["addons", "foo.gd", "bar"]). The first two segments match, then the loop tries to read script_path_parts[2], which doesn't exist. Skip directories that have more segments than the script path, since they can never match as a prefix. * fix(plugin): skip and warn on blank format on save ignored directories entries Fixes a bug introduced in #191 An ignored directory entry that is blank, "res://", or just "/" would normalize to an empty string. Splitting that on "/" produces [""], which could be compared against script path segments in unintended ways instead of being treated as an invalid entry. Skip these entries and push a warning so users know the entry has no usable path and needs to be removed or corrected. Also warns the user that if the intention was to ignore the whole project that this is not supported here and they need to turn of format on save instead. * fix(plugin): warn on format on save ignored directories entries that duplicate an earlier entry after normalization Fixes a bug introduced in #191 The Format on Save Ignored Directories list was not checked for entries that normalize to the same path despite being written differently (e.g. "res://addons" and "addons/"). This could let redundant entries sit in the list unnoticed. Track each normalized entry as it's validated, and if a later entry normalizes to the same as one already seen, skip it and push a warning naming both the duplicate and the original entry it conflicts with. * refactor: trim comments, rename variables --------- Co-authored-by: Nathan Lovato <12694995+NathanLovato@users.noreply.github.com>
1 parent b7e122c commit 3982a8c

1 file changed

Lines changed: 34 additions & 10 deletions

File tree

addons/GDQuest_GDScript_formatter/plugin.gd

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func _enter_tree() -> void:
173173
installer = FormatterInstaller.new(formatter_cache_dir)
174174
add_child(installer)
175175
installer.installation_completed.connect(
176-
func _on_installation_completed (binary_path: String) -> void:
176+
func _on_installation_completed(binary_path: String) -> void:
177177
set_editor_setting(SETTING_FORMATTER_PATH, binary_path)
178178
_has_formatter_command = has_command(binary_path)
179179
if not _has_formatter_command:
@@ -188,7 +188,7 @@ func _enter_tree() -> void:
188188
menu.update_menu(true),
189189
)
190190
installer.installation_failed.connect(
191-
func _on_installation_failed (error_message: String) -> void:
191+
func _on_installation_failed(error_message: String) -> void:
192192
push_error("Formatter installation failed: ", error_message),
193193
)
194194

@@ -347,18 +347,42 @@ func _on_resource_saved(saved_resource: Resource) -> void:
347347
if not do_format_on_save and not lint_on_save:
348348
return
349349

350-
var ignored_directories = get_editor_setting(SETTING_IGNORED_DIRECTORIES)
351-
var path = script.resource_path.trim_prefix("res://")
350+
var ignored_directories_normalized: Array[String] = []
351+
var ignored_directories_seen: Dictionary[String, String] = { }
352+
for directory: String in get_editor_setting(SETTING_IGNORED_DIRECTORIES):
353+
# We split paths on "/", we trim trailing slashes to avoid empty path
354+
# segments that would never match when checking ignored directories.
355+
var directory_normalized := directory.trim_prefix("res://").trim_suffix("/")
356+
if directory_normalized.is_empty():
357+
push_warning(
358+
"GDScript Formatter: Format on Save Ignored Directories entry \"%s\" " % directory
359+
+ "has no path after removing \"res://\" and trailing slashes, and will be skipped. "
360+
+ "This may mean you're trying to ignore the entire project, which isn't supported here. "
361+
+ "Please remove it from the list, enter a valid path, or turn off format on save instead."
362+
)
363+
continue
364+
365+
if ignored_directories_seen.has(directory_normalized):
366+
push_warning(
367+
"GDScript Formatter: Format on Save Ignored Directories entry \"%s\" " % directory
368+
+ "refers to the same directory as entry \"%s\" "
369+
% ignored_directories_seen[directory_normalized]
370+
+ "and will be skipped. Please remove the duplicate entry from the list."
371+
)
372+
continue
352373

353-
var script_path_parts := path.split("/")
374+
ignored_directories_seen[directory_normalized] = directory
375+
ignored_directories_normalized.push_back(directory_normalized)
354376

355-
for directory: String in ignored_directories:
356-
var normalized_dir := directory.trim_prefix("res://")
357-
var directory_parts := normalized_dir.split("/")
377+
var saved_script_path_segments := script.resource_path.trim_prefix("res://").split("/")
378+
for ignored_directory_path: String in ignored_directories_normalized:
379+
var ignored_directory_segments := ignored_directory_path.split("/")
380+
if ignored_directory_segments.size() > saved_script_path_segments.size():
381+
continue
358382

359383
var matches := true
360-
for i in range(directory_parts.size()):
361-
if directory_parts[i] != script_path_parts[i]:
384+
for i in range(ignored_directory_segments.size()):
385+
if ignored_directory_segments[i] != saved_script_path_segments[i]:
362386
matches = false
363387
break
364388

0 commit comments

Comments
 (0)