Bug 213904

Summary: [maintainer] [patch] games/zdoom: Add desktop and pixmap files
Product: Ports & Packages Reporter: Kyle Evans <kevans>
Component: Individual Port(s)Assignee: Jan Beich <jbeich>
Status: Closed FIXED    
Severity: Affects Only Me CC: jbeich
Priority: --- Keywords: patch
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
svn(1) diff of games/zdoom
kevans: maintainer-approval+
svn(1) diff of games/zdoom
kevans: maintainer-approval+
svn(1) diff of games/zdoom kevans: maintainer-approval+

Description Kyle Evans freebsd_committer freebsd_triage 2016-10-30 04:35:29 UTC
Created attachment 176297 [details]
svn(1) diff of games/zdoom

Hello!

For the sake of those of us that do like desktop shortcuts with icons resembling the official distributions icons, add a pixmap and .desktop file.

Portlint and Poudriere runs are both successful with no red flags.
Comment 1 Jan Beich freebsd_committer freebsd_triage 2016-11-08 05:23:49 UTC
Comment on attachment 176297 [details]
svn(1) diff of games/zdoom

> +	${CP} ${FILESDIR}/zdoom.desktop ${STAGEDIR}${PREFIX}/share/applications
> +	${CP} ${FILESDIR}/zdoom.xpm ${STAGEDIR}${PREFIX}/share/pixmaps

Convert to INSTALL_DATA.

> +++ files/zdoom.desktop	(working copy)
> @@ -0,0 +1,8 @@
> +#!/usr/bin/env xdg-open

Why do you need this line?

> +[Desktop Entry]
> +Version=1.0
> +Type=Application
> +Name=ZDoom
> +Exec=zdoom
> +Icon=zdoom
> +Categories=Game;
 
For simple .desktop under files/ directory consider defining DESKTOP_ENTRIES in Makefile instead.
Comment 2 Kyle Evans freebsd_committer freebsd_triage 2016-11-08 14:37:50 UTC
Created attachment 176787 [details]
svn(1) diff of games/zdoom

Addressing concerns from jbeich@:

* Converted ${CP} to ${INSTALL_DATA} for the XPM
* Used DESKTOP_ENTRIES instead of a custom .desktop file

The shebang for the .desktop script, as far as I'm aware, is for potentially oddball WMs that wouldn't necessarily execute the .desktop properly. That being said, I don't have any strong feelings towards keeping it in given that DESKTOP_ENTRIES doesn't bother including it -- I've gone ahead and converted the .desktop file to its equivalent DESKTOP_ENTRIES entry, to be a bit more standard.
Comment 3 Jan Beich freebsd_committer freebsd_triage 2016-11-09 04:31:18 UTC
Comment on attachment 176787 [details]
svn(1) diff of games/zdoom

> +DESKTOP_ENTRIES=	"ZDoom" "${COMMENT}" "${PORTNAME}" "${PORTNAME}" "Game;" ${FALSE}

Don't use ${FALSE} variable if the value isn't supposed to be a command (e.g. /usr/bin/false) or sh(1) builtin. Besides, "false" isn't actually default behavior for StartupNotify:

  If true, it is KNOWN that the application will send a "remove"
  message when started with the DESKTOP_STARTUP_ID environment
  variable set. If false, it is KNOWN that the application does not
  work with startup notification at all (does not shown any window,
  breaks even when using StartupWMClass, etc.). If absent, a
  reasonable handling is up to implementations (assuming false, using
  StartupWMClass, etc.). (See the Startup Notification Protocol
  Specification for more details).

https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s05.html

Also, some fields can be left empty according to Mk/bsd.port.mk:

  * Comment, Icon and StartupNotify may be empty
    strings (""). Categories may be an empty string in some
    cases (see below). The other fields are mandatory.
  * If Categories is an empty string, bsd.port.mk will try
    to deduce a default value using the CATEGORIES variable.
    If the deduction fails, you will have to set Categories
    manually. You should check the generated value using
    "make desktop-categories", and override it if necessary.

which would result in

  DESKTOP_ENTRIES=	"ZDoom" "" "" "${PORTNAME}" "" ""

but you may want to clarify category as "Game;Shooter;" per
https://standards.freedesktop.org/menu-spec/latest/apas02.html
Comment 4 Jan Beich freebsd_committer freebsd_triage 2016-11-09 04:36:20 UTC
Sorry, empty Icon means the field is omitted from the generated .desktop file.
Comment 5 Kyle Evans freebsd_committer freebsd_triage 2016-11-09 22:48:45 UTC
Created attachment 176836 [details]
svn(1) diff of games/zdoom

My apologies, very much new to this still. =)

* 'false' => ${FALSE} I was admittedly unsure of, but portlint doesn't seem to distinguish between using false as a string and false as a command, because the original iteration used it as a string.

* I've removed the fields where the default behavior is acceptable, clarified the category as "Games;Shooter;" as mentioned because we seem to get this specific elsewhere.

* I left the icon string in place, because I like the icon and went through the effort to convert it to .xpm. =)
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-11-09 23:00:05 UTC
A commit references this bug:

Author: jbeich
Date: Wed Nov  9 22:59:06 UTC 2016
New revision: 425815
URL: https://svnweb.freebsd.org/changeset/ports/425815

Log:
  games/zdoom: add .desktop file + icon for it

  PR:		213904
  Submitted by:	Kyle Evans <bsdports@kyle-evans.net> (maintainer)

Changes:
  head/games/zdoom/Makefile
  head/games/zdoom/files/
  head/games/zdoom/files/zdoom.xpm
Comment 7 Jan Beich freebsd_committer freebsd_triage 2016-11-09 23:01:47 UTC
Thanks. Landed.