Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite the link fixer script #3

Merged
merged 3 commits into from
Nov 18, 2024
Merged

Rewrite the link fixer script #3

merged 3 commits into from
Nov 18, 2024

Conversation

Wuestengecko
Copy link
Member

This commit greatly refactors the link fixer script. The previous solution was based on detecting links using regular expressions and applying some pattern-based fixes. However, in practice, this approach proved to be a bad one. Luckily it at least didn't make things worse (the model was already broken before anyways), but often enough it didn't produce the desired outcome of a no-longer-broken model.

The new implementation introduced in this commit loads the model using capellambse's MelodyLoader facility, and applies fixes using the semantics-aware methods that the loader offers. Specifically, it tries to parse all XML attributes as a list of element links, discards the (potentially broken) file path from each link, and finally regenerates them using MelodyLoader.create_link.

BREAKING CHANGE: Changed the hook's ID to fix-capella-fragment-links. Experimentation has unfortunately shown the new implementation to be noticably slower than the old one - to remedy this, the hook now skips itself during the follow-up commit, so as to not run twice. The ID change was made to more reliably identify the hook, and reduce the chance for name clashes with a different hook that might fix some other type of link in some other type of file.

@Wuestengecko Wuestengecko force-pushed the refactor-fix-links branch 2 times, most recently from 634fe2e to 0010a21 Compare November 18, 2024 12:49
This commit greatly refactors the link fixer script. The previous
solution was based on detecting links using regular expressions and
applying some pattern-based fixes. However, in practice, this approach
proved to be a bad one. Luckily it at least didn't make things worse
(the model was already broken before anyways), but often enough it
didn't produce the desired outcome of a no-longer-broken model.

The new implementation introduced in this commit loads the model using
capellambse's MelodyLoader facility, and applies fixes using the
semantics-aware methods that the loader offers. Specifically, it tries
to parse all XML attributes as a list of element links, discards the
(potentially broken) file path from each link, and finally regenerates
them using `MelodyLoader.create_link`.

BREAKING CHANGE: Changed the hook's ID to `fix-capella-fragment-links`.
Experimentation has unfortunately shown the new implementation to be
noticably slower than the old one - to remedy this, the hook now skips
itself during the follow-up commit, so as to not run twice. The ID
change was made to more reliably identify the hook, and reduce the
chance for name clashes with a different hook that might fix some other
type of link in some other type of file.
Copy link
Member

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ewuerger ewuerger merged commit cd25e42 into main Nov 18, 2024
6 checks passed
@ewuerger ewuerger deleted the refactor-fix-links branch November 18, 2024 15:39
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