@b4n requested changes on this pull request.

I am not sure we do want this, but maybe.  I don't see much downside, but 
perhaps when building with `--enable-binreloc` (gives `Exec=//bin/geany` which 
is… well, both right an wrong), yet I don't know how much sense a .desktop 
makes for a relocatable installation.
So let's say we're happy with it.

However, I don't like the implementation.  IMO, we should rather do something 
like 
https://github.com/b4n/geany/commit/884309bd6e7969f7b2d01592fa6741cd76b1580b -- 
and AFAIK it's how other do.

This said, full path is not a popular trend in my Debian stable, which kind of 
makes me wonder if there is a downside I can't think about:
```shell
$ rgrep 'Exec *= */' /usr/share/applications/ | wc -l # absolute paths
27
$ rgrep 'Exec *= *[^/]' /usr/share/applications/ | wc -l # non-absolute paths
233
```

> @@ -11,6 +11,18 @@ AC_DEFUN([GEANY_PREFIX],
        if test "x$exec_prefix" = xNONE; then
                exec_prefix=$prefix
        fi
+
+       # The $bindir variable is equal to the literal string "${exec_dir}/bin"
+       # rather than the actual path. pkgconfig (.pc) files can use that ok, 
but
+       # other files written out by configure require the literal path.
+       #
+       # If the --bindir argument is given to configure, then $bindir will 
already
+       # be equal to an absolute path.

Actually no, AFAIK it's valid to pass `--bindir` with placeholders.  The only 
"reliable" way of expanding one of the dir variables is to eval it until it 
stops changing -- which is both annoying and tricky.  IIRC there's a 3rd party 
macro to do so, but in any case it's not great.

The best solution usually is to perform these replacements at make time.

> @@ -9,6 +9,9 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([1.11 -Wall parallel-tests subdir-objects])
 AC_CONFIG_HEADERS([config.h])
 
+PKG_DESC="A fast and lightweight IDE using GTK+"

I think that's kind of over-engineering the thing, and IMO should at least not 
be part of this change.

> +Name: @PACKAGE_NAME@
+Description: @PKG_DESC@

This is unrelated to the proposed change

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2728#pullrequestreview-569935949

Reply via email to