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

Set SingleMainWindow in Desktop Entry #2387

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MattSturgeon
Copy link
Contributor

Follow up to #2386, this tells the DE that we don't need a "New Window" button because we typically only have one single "main" window.

SingleMainWindow is a hint to DEs for whether to offer a "New Window" button.

image

This makes sense for NexusMods.App, because it only supports having one "main" window open.


The CI is unhappy about SingleMainWindow:

error: file contains key "SingleMainWindow" in group "Desktop Entry", but keys extending the format should start with "X-"

https://github.com/Nexus-Mods/NexusMods.App/actions/runs/12342433935/job/34442250123?pr=2386#step:8:1375

The CI is validating against Desktop Entry Spec v1.0, however SingleMainWindow was added in v1.5.

I'm not sure if we can have this as an X- prefixed option and it still work, need to test that out. Otherwise, is it possible for the linter to support a newer Desktop Entry Spec version?

Copy link
Member

@erri120 erri120 left a comment

Choose a reason for hiding this comment

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

Change is fine, I'll try and find out if we can upgrade to a newer spec version, I think I ran into this issue before.

Copy link
Member

@Sewer56 Sewer56 left a comment

Choose a reason for hiding this comment

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

This is good for the time being.

Long term we are supposed to expose multi window (in fact, we previously did); but opted to disable it for the time being due to some technical caveats.

@github-actions github-actions bot added the status-needs-rebase Set by CI do not remove label Dec 16, 2024
Copy link
Contributor

This PR conflicts with main. You need to rebase the PR before it can be merged.

This indicates to the DE that the app usually runs with a single "main"
window, and doesn't support opening additional instances.

This can be used by desktops to disable "New Window" actions in the
right-click menu.
@MattSturgeon MattSturgeon force-pushed the desktop-entry-single-window branch from 7e3224e to 99f2a19 Compare December 16, 2024 10:16
@github-actions github-actions bot removed the status-needs-rebase Set by CI do not remove label Dec 16, 2024
Copy link
Contributor

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

@erri120
Copy link
Member

erri120 commented Dec 16, 2024

@Sewer56 https://github.com/Sewer56/PupNet-Deploy/tree/main/PupNet/Assets it looks like we have appimagetool builds in the deploy project that are outdated. I think we can just upgrade that.

@Sewer56
Copy link
Member

Sewer56 commented Dec 16, 2024

@erri120 Sounds good to me.
I don't have Linux set up here are the office; I can install it, but might take a bit.
Wanna PR it instead? Then we can PR that to upstream; if upstream's willing to accept it (if you remember, upstream stopped maintaining for time being).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

3 participants