Fix localized path for action scripts #143

mkh-user · 2025-12-03T06:43:31Z

Summary of Changes

Adds globalization for action scripts paths for correct loading. Before this, in core/main.gd:380 local and global comparison always lead to false and leads to empty item for all action scripts, this breaks startup.

Related Items

Refactor for 0.2 Release: Code Style, Action Scripts, and Bug Fixes #139: Where this bug originated.

Technical Details & Testing

Adds globalization for path generation, also checks for empty item values and improves readability with changing .remove_char(46) to .remove_chars(".").

Checklist

Linked all relevant issues/PRs/discussions
Code follows project style and guidelines
Self-reviewed and tested thoroughly
Documentation updated (if applicable)
No new warnings or errors introduced
Relevant changes added to CHANGELOG

Summary by CodeRabbit

Bug Fixes
- Now notifies when a script path cannot be matched to a menu item and exits early to avoid errors.
- Improved script name sanitization to correctly handle names containing multiple dots.
- Standardized script path handling to ensure consistent path formatting.
Documentation
- Changelog updated describing the script path globalization fix.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-03T06:43:44Z

📝 Walkthrough

Walkthrough

On script connection, emit a Global.ERROR notification when no menu item matches the script; sanitize assigned script names by removing all . characters; and wrap computed script paths with S.globalize_path to standardize path formatting in core/main.gd.

Changes

Cohort / File(s)	Change Summary
Script connection, name sanitization & path globalization `core/main.gd`	Emit a `Global.ERROR` notification and exit early when a script path doesn't match any menu item; change name sanitization to remove all `.` characters; wrap computed script paths with `S.globalize_path` for standardized paths.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main change: fixing path localization for action scripts, which is the core issue addressed in core/main.gd.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch action-scripts-path-globalization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mkh-user · 2025-12-03T06:44:45Z

@coderabbitai, commit additional item to CHANGELOG.md file for this PR here.

coderabbitai · 2025-12-03T06:45:31Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the CHANGELOG.md file to understand its format
cat CHANGELOG.md

Length of output: 18479

@mkh-user, I cannot commit code directly to the repository. However, I can provide you with the changelog entry that should be added to the CHANGELOG.md file.

Here's the entry that should be added under the ### Fixed section in the [unreleased] block:

- Action scripts loading broken due to incorrect path globalization in `core/main.gd` ([#143](https://github.com/text-forge/text-forge/pull/143))

This entry should be placed in the ### Fixed section under [unreleased] in the CHANGELOG.md file. The fix addresses the bug where local and global path comparison always returned false, causing empty item values for all action scripts and breaking startup.

Would you like me to provide more details about where exactly to place this entry in the file?

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8226de9 and f76e836.

📒 Files selected for processing (1)

core/main.gd (3 hunks)

🔇 Additional comments (2)

core/main.gd (2)

492-494: Core fix correctly addresses the path comparison issue.

Globalizing the path at the source ensures consistency between path generation (line 209) and script connection (line 380). This resolves the bug where local paths failed to match the globalized paths returned by the resource loader.

Note: Line 211 now applies S.globalize_path() to an already-globalized path, but since globalize_path() is idempotent, this is safe. Optionally, you could remove the redundant call at line 211 for clarity.

402-402: > Likely an incorrect or invalid review comment.

core/main.gd

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

core/main.gd (1)

386-391: Early return correctly prevents configuring scripts for unmatched paths.

This implements the previously requested early return: when no menu item matches path, you now emit an error and exit before using the empty item dictionary, avoiding bogus script nodes and potential runtime issues. LGTM.

🧹 Nitpick comments (1)

core/main.gd (1)
201-220: Path globalization fix looks good; consider dropping now‑redundant S.globalize_path call.

Standardizing _get_script_path_for_item to return S.globalize_path(...) resolves the local vs global mismatch when comparing _get_script_path_for_item(option) to path in _connect_script, and keeps the same normalized base as used for script.name.

Given that change, script_path in _load_scripts is already globalized, so the extra S.globalize_path(script_path) in the FileAccess.file_exists check is redundant and can be simplified:
-			var script_path := _get_script_path_for_item(item)
+			var script_path := _get_script_path_for_item(item)
 			# Disable items without script (except submenu roots)
-			if not FileAccess.file_exists(S.globalize_path(script_path)):
+			if not FileAccess.file_exists(script_path):
 				if item.has("popup") and item.get("type", OptionTypes.REGULAR) != OptionTypes.SUBMENU:
 					item.get("popup").set_item_disabled(item.get("popup").get_item_index(item.get("code", 0)), true)
Not functionally broken as‑is, but this would tighten things up and make the data flow a bit clearer.

Also applies to: 380-381, 491-495

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f1d04e and 97b66d6.

📒 Files selected for processing (1)

core/main.gd (3 hunks)

🔇 Additional comments (1)

core/main.gd (1)

403-403: script.name sanitization is clearer and matches path generation.

Using to_snake_case().remove_chars(".") makes the intent explicit and keeps the runtime script name aligned with the normalized base used for the script path. This is a straightforward readability and consistency win.

Fix localized path for action scripts

f76e836

github-project-automation bot added this to Release: Text Forge 1.0 Dec 3, 2025

github-project-automation bot moved this to Needs Review in Release: Text Forge 1.0 Dec 3, 2025

mkh-user self-assigned this Dec 3, 2025

Update CHANGELOG.md

9f1d04e

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai bot reviewed Dec 3, 2025

View reviewed changes

core/main.gd Show resolved Hide resolved

Add early return to prevent operating on empty item

97b66d6

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

coderabbitai bot reviewed Dec 3, 2025

View reviewed changes

mkh-user added action scripts Official action scripts or action scripts features bug A confirmed issue causing incorrect behavior or unexpected results labels Dec 3, 2025

mkh-user moved this from Needs Review to In Progress in Release: Text Forge 1.0 Dec 3, 2025

mkh-user added this to the Text Forge 0.2 milestone Dec 3, 2025

mkh-user merged commit ca7401c into Main Dec 3, 2025
2 checks passed

mkh-user deleted the action-scripts-path-globalization branch December 3, 2025 06:56

github-project-automation bot moved this from In Progress to Completed in Release: Text Forge 1.0 Dec 3, 2025

coderabbitai bot mentioned this pull request Dec 22, 2025

Add support for normalizing line endings and related features #171

Merged

7 tasks

mkh-user mentioned this pull request Dec 30, 2025

Cleanup: drop action scripts, panels, and translations from exports #176

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix localized path for action scripts #143

Fix localized path for action scripts #143

Uh oh!

mkh-user commented Dec 3, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 3, 2025 •

edited

Loading

Walkthrough

Changes

Uh oh!

mkh-user commented Dec 3, 2025

Uh oh!

coderabbitai bot commented Dec 3, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix localized path for action scripts #143

Fix localized path for action scripts #143

Uh oh!

Conversation

mkh-user commented Dec 3, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Related Items

Technical Details & Testing

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Pre-merge checks and finishing touches

Uh oh!

mkh-user commented Dec 3, 2025

Uh oh!

coderabbitai bot commented Dec 3, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mkh-user commented Dec 3, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 3, 2025 •

edited

Loading