Fix format on save ignored directories path matching logic - #325
Merged
NathanLovato merged 5 commits intoAug 14, 2026
Conversation
NoahGreer
marked this pull request as draft
August 12, 2026 21:30
Contributor
Author
|
@NathanLovato there were no parse errors when I originally authored these changes, but I rebased this branch on |
NoahGreer
force-pushed
the
noahg/fix-format-on-save-ignored-directories-path-matching-logic
branch
from
August 14, 2026 04:46
dc2d7c3 to
1572f66
Compare
Contributor
|
Thanks for taking the time to work on this. Note moving forward, feel free to directly patch things + do not worry about being blamed for a mistake I introduced in the code. I patched the issue in main, please let me know when you'd like me to merge your changes. |
…ng slash Fixes a bug introduced in GDQuest#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.
…eeper than script path Fixes a bug introduced in GDQuest#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.
…s entries Fixes a bug introduced in GDQuest#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.
NoahGreer
force-pushed
the
noahg/fix-format-on-save-ignored-directories-path-matching-logic
branch
from
August 14, 2026 07:51
1572f66 to
58453ad
Compare
NoahGreer
marked this pull request as ready for review
August 14, 2026 08:08
Contributor
Author
|
Thank you for the update and the guidance on how to handle this scenario. I was less concerned with being blamed and more concerned with trying to avoid making a slightly confusing situation more confusing for you and other contributors. Thanks for patching that error. I have now rebased my changes on |
…duplicate an earlier entry after normalization Fixes a bug introduced in GDQuest#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.
NoahGreer
force-pushed
the
noahg/fix-format-on-save-ignored-directories-path-matching-logic
branch
from
August 14, 2026 08:28
58453ad to
16ca277
Compare
Contributor
|
Thank you very much for your help! |
NathanLovato
added a commit
that referenced
this pull request
Aug 14, 2026
* 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>
NoahGreer
deleted the
noahg/fix-format-on-save-ignored-directories-path-matching-logic
branch
August 14, 2026 09:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements:
Does this PR introduce a breaking change?
No.
New feature or change
What is the current behavior?
The logic introduced in #191 that checks a script's path against the user's ignored directories list has several bugs:
An ignored directory entry with a trailing slash produces a trailing empty string when split (e.g.
"addons/"->["addons", ""]), which can never match a real path segment, so the entry is silently ignored. This means that the default ignored path"addons/"introduced in the above PR does not actually work as expected and files in that directory were still being formatted on save.If an ignored directory entry has more path segments than the script's own path, the matching loop indexes past the end of the script's path parts array (e.g. checking
res://addons/foo.gdagainst an ignored entry ofaddons/foo.gd/bar) and produces an index out-of-bounds error.Blank entries in the ignored directories list (
"","res://", or"/") are not handled, and can be compared as an empty path segment in unintended ways.Because matching returns as soon as one entry matches, any blank entries later in the list are never checked, so there was no way to warn a user about them.
Entries that normalize to the same directory despite being written differently (e.g.
"res://addons"and"addons/") are not detected, so redundant entries can sit in the list unnoticed.What is the new behavior?
Trailing slashes are trimmed from each ignored directory entry before splitting, so these entries match as expected.
Ignored directory entries with more path segments than the script's path are skipped, since they can never match as a prefix, avoiding the out-of-bounds index access in the path parts matching loop.
Blank entries are detected and skipped, and a warning is pushed telling the user the entry has no usable path, that it may indicate an attempt to ignore the whole project. Ignoring the whole project this way does not make sense when they could simply turn off format on save. Now the user is warned to remove the entry, fix the path, or turn off format on save instead.
Validation of all entries (trimming, blank checks, warnings) is now done in a separate pass before any matching happens, so every entry is always checked and warned about regardless of order, rather than stopping early once a match is found.
Entries that normalize to the same directory as an earlier entry are detected, skipped, and warned about, naming both the duplicate and the original entry it conflicts with.