Skip to content

Jeremypw/plugins/fix libpeas2 #1566

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

Merged
merged 7 commits into from
May 28, 2025
Merged

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented May 25, 2025

Fixes #1560

In PluginManager:

  • Make namespaces explicit
  • Make ExtensionSet a construct property

In master, plugins are getting deactivated as soon as loaded for reasons that are obscure. After much trial, error and elimination it appears that the above two changes fix the issue.

View the diff hiding whitespace changes.

@jeremypw jeremypw marked this pull request as ready for review May 25, 2025 12:40
@jeremypw jeremypw requested a review from a team May 25, 2025 12:40
@jeremypw jeremypw mentioned this pull request May 26, 2025
11 tasks
@jeremypw jeremypw added the Priority: Critical e.g. security implications or reproducible crashing label May 27, 2025
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Definitely fixes the issue! A couple nitpicks

Comment on lines 60 to 61
public Peas.Engine engine { get; construct; }
public Peas.ExtensionSet extension_set {get; construct; }
Copy link
Member

Choose a reason for hiding this comment

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

These should not be construction properties. Only things passed into an Object call should be construct. Properties that are set inside the class should be private set: https://docs.vala.dev/tutorials/programming-language/main/03-00-object-oriented-programming/03-14-gobject-style-construction.html

I think extension_set can just be private. It looks like maybe we need to hold a reference to it (might be a good place for a comment), but I don't think it needs to be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, ok. I tend to use construct; for any property that is assigned once only upon construction. Sure, if its private set; we can ensure that anyway so I guess its unnecessary. I'll see if it still works with private set; properties.

jeremypw added 2 commits May 28, 2025 10:56
Use `construct;` only for properties assigned in Object construction
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

🚀

@danirabbit danirabbit merged commit 756f83c into master May 28, 2025
6 checks passed
@danirabbit danirabbit deleted the jeremypw/plugins/fix-libpeas2 branch May 28, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical e.g. security implications or reproducible crashing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bracket completion no longer works
2 participants