[yan12125]

See: https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html

• Address review comments

Seems #296 is stale, so I created a replacement with fixes. Differences from the previous pull request are in the second commit.

[rom1v]

Hi,

Thank you 👍

I added a first commit to replace the hardcoded share/ in meson.build by the datadir variable (on dev branch). Then I rebased/re-split the commits.

Then I added a scrcpy-console.desktop to provide a launcher with a terminal (like scrcpy-console.bat).

The resulting branch is here: pr3351

Please review and test 😉

[yan12125]

Awesome improvements! Just one minor suggestion -

For some users, adb is not in default $PATH but the one configured in .bashrc/ or .zshrc/...

"or" indeed makes it clearer, but a slash after .bashrc is somewhat unusual. Probably better with

For some users, adb is not in default $PATH but the one configured in .bashrc, .zshrc, or other shell startup scripts.

(Term "shell startup scripts" is just the first one out of my head. Different people use different terms for those files :D)

scrcpy.desktop works as usual. For scrcpy-console.desktop, the "complex" Exec= line hits some other software. I'm using QTerminal on LXQt, where lxqt/qterminal#961 is needed to correctly parse that line. That's a QTerminal bug, so keeping the current scrcpy-console.desktop is fine.

[rom1v]

(Term "shell startup scripts" is just the first one out of my head. Different people use different terms for those files :D)

In man bash:

       ~/.bashrc
              The individual per-interactive-shell startup file

So "shell startup script" looks good to me.

For some users, adb is not in default $PATH but the one configured in .bashrc, .zshrc, or other shell startup scripts.

I suggest:

+# For some users, the PATH or ADB environment variables are set from the shell
+# startup file, like .bashrc or .zshrc… Run an interactive shell to get
+# environment correctly initialized.

What do you think? See pr3351.2.

For scrcpy-console.desktop, the "complex" Exec= line hits some other software. I'm using QTerminal on LXQt, where lxqt/qterminal#961 is needed to correctly parse that line.

😞

That's a QTerminal bug, so keeping the current scrcpy-console.desktop is fine.

OK

[yan12125]

Looks great, thank you very much for the efforts!

[rom1v]

@asnelling as the author of #296, any remark?

[asnelling]

For system-wide installations, Exec should use the system-wide adb installed at, e.g.: /usr/bin/adb and leave customization concerns up to the user in e.g.: ~/.local/applications/scrcpy.desktop
👍 good to merge

[asnelling]

IMO, Exec= should let the system resolve scrcpy and not invoke a shell. Users customizations belong in ~/.local/share/applications/scrcpy.desktop . It seems the convention (at least in Ubuntu) is Exec=scrcpy or Exec=/usr/bin/scrcpy, which you can see with:

$ grep '^Exec=' /usr/share/applications/*.desktop

More importantly, merge it 👍

[yan12125]

For system-wide installations, Exec should use the system-wide adb installed at, e.g.: /usr/bin/adb

It's up to whether rom1v wants to support $PATH or $ADB configured in .bashrc/.zshrc/... or not. I'm fine with either.

It seems the convention (at least in Ubuntu) is Exec=scrcpy or Exec=/usr/bin/scrcpy

It's also possible to extract those "complex" Exec= lines to standalone scripts like /usr/bin/scrcpy-console.sh and /usr/bin/scrcpy-noconsole.sh, and use them in Exec=.

[rom1v]

IMO, Exec= should let the system resolve scrcpy and not invoke a shell.

So, for the non-console version:

Exec=/usr/bin/scrcpy

For the console version:

Exec=/usr/bin/scrcpy || bash -i -c 'read -p "Press any key to quit..."'

?

Where should the user define ADB or PATH so that it is used for the launcher in that case?

@yangfl any opinion about the content of Exec=?

[yangfl]

I think you can't use the shell syntax in Exec=.

"$SHELL" -i -c scrcpy

Strange, but acceptable as long as the software author satisfies with it.

[yan12125]

Exec=/usr/bin/scrcpy || bash -i -c 'read -p "Press any key to quit..."'

I think you can't use the shell syntax in Exec=.

Yeah, || is passed as an argument and failed. Here is a working example with waiting and no interactive shell:

Exec=/bin/bash -c 'scrcpy || read -p "Press any key to quit..."'

"$SHELL" -i -c scrcpy
Strange, but acceptable as long as the software author satisfies with it.

I checked and found that qterminal and similar programs expand environment variables. I'm not sure if other terminal emulators do that or not.

Where should the user define ADB or PATH so that it is used for the launcher in that case?

That depends on how an X11 or Wayland session is started. Apparently there is no universal way - see https://wiki.archlinux.org/title/Environment_variables#Graphical_environment

[rom1v]

Yeah, || is passed as an argument and failed. Here is a working example with waiting and no interactive shell:

Exec=/bin/bash -c 'scrcpy || read -p "Press any key to quit..."'

OK. But then the scrcpy-console version will load environment variables from .bashrc (whatever the default shell), while the non-console version will not.

"$SHELL" -i -c scrcpy

Strange, but acceptable as long as the software author satisfies with it.

OK, so maybe I will keep pr3351.2.

Thank you.

[yan12125]

But then the scrcpy-console version will load environment variables from .bashrc (whatever the default shell), while the non-console version will not.

Yeah that's an issue. How about /bin/bash --norc --noprofile ...?

[yan12125]

How about /bin/bash --norc --noprofile ...?

Assuming no objection for this approach, I imported pr3351.2 branch, added proposed bash arguments and force-updated this PR.

[rom1v]

Merged into dev.

[yan12125]

Thanks!