Skip to content

Conversation

@op200
Copy link

@op200 op200 commented Dec 8, 2025

#446

This bug involves low-level class, and the style editor's renaming function is a commonly used feature

I think instead of ignoring it, it's better to provide a temporary solution and wait until the underlying code is modified in the future to make the changes, in order to prevent many tragedies about subtitle modifications

@arch1t3cht
Copy link
Member

Sorry, but while I understand wanting a solution to this issue now instead of waiting a long time for parsing to be reworked from scratch, I don't think adding even more subtly flawed parsing code improves the situation.
(For example, the comments already say that this doesn't support syntax like \r(StyleName), but what it also does not support is \ rStyleName, which is actually also valid. Also, it runs into edge cases when styles contain backslashes or parentheses.)

I think a much easier way to reduce the fallout of #446 is to make the style replacer only edit lines in which it actually finds \r tags. (Whereas at the moment it parses and updates every single line). If that turns out to not be enough, another workaround would be to add a third button to the style replacer prompt, like "Do you want to change all instances of this style in the script to this name?" - "No" / "Cancel" / "Yes, only update style fields" / "Yes".

@arch1t3cht arch1t3cht closed this Dec 11, 2025
@op200
Copy link
Author

op200 commented Dec 11, 2025

I think adding a third button would be more appropriate, but I don't know GUI related code

Perhaps I can increase its compatibility with style tag like \ r ( StyleName

@arch1t3cht
Copy link
Member

No, that's exactly the problem - that would spiral into a full tag parser implemented just for this one use case, and I really don't want that. The long-term solution there is to write a better (in particular idempotent) tag parser for all of Aegisub, but that will need to wait until at least after 3.5.

Really, I think making the updater only update lines where it actually found \r tags is the simplest solution and will probably fix the vast majority of cases. I can implement that at some point if nobody else pr's it first. If that turns out to not be enough, I can add a third button.

@op200
Copy link
Author

op200 commented Dec 11, 2025

Thank you for your explanation, I understand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants