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

Desktop Integration #42

Merged
merged 8 commits into from
Feb 9, 2018
Merged

Desktop Integration #42

merged 8 commits into from
Feb 9, 2018

Conversation

lanodan
Copy link
Contributor

@lanodan lanodan commented Dec 7, 2017

Pull request for #31 (better than commit ids in the issue thread I hope/guess)

@fmang fmang added this to the 1.7 milestone Feb 5, 2018
Copy link
Owner

@fmang fmang left a comment

Choose a reason for hiding this comment

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

Now that we've entered 1.7, I'm gonna merge this as soon as it's ready. We're almost there.

oshu.desktop Outdated
Exec=oshu %F
MimeType=text/x-osu
Terminal=true
Categories=Game;X-Music;X-Rythm
Copy link
Owner

Choose a reason for hiding this comment

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

I'm surprised this file still exists. Isn't it moved to share/?

Exec=oshu %F
MimeType=text/x-osu
Terminal=true
Categories=Game;X-Music;X-Rhythm
Copy link
Owner

Choose a reason for hiding this comment

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

The bang in the filename may cause trouble for some systems. Calling it oshu without the bang is consistent with the executable file, so I'd rather play it safe.

x-osu.xml Outdated
</magic>
<glob pattern="*.osu"/>
</mime-type>
</mime-info>
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove that one, it's was moved.

share/x-osu.xml Outdated
</magic>
<glob pattern="*.osu"/>
</mime-type>
</mime-info>
Copy link
Owner

Choose a reason for hiding this comment

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

I know I'm the one that suggested x-osu, but since there are other file types for osu!, rather than naively name them x-osb, x-osk, maybe x-osu-something is better.

Here, that would be text/x-osu-beatmap. What do you think?

You're right for text/ vs application/, application is binary according to MDN.

Copy link
Owner

Choose a reason for hiding this comment

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

Almost there! The file should be renamed x-osu-beatmap.xml

@lanodan
Copy link
Contributor Author

lanodan commented Feb 9, 2018

It should be okay now. (I have no idea if you get notifications on pushes)

@fmang fmang merged commit 9b5f7fd into fmang:master Feb 9, 2018
@fmang
Copy link
Owner

fmang commented Feb 9, 2018

Actually I did receive the push notification, but didn't have time to merge.

Thank you for the contribution!

I'll add the autoconf rules next.

@fmang fmang self-assigned this Feb 9, 2018
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