Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Po Lu]

Stefan Kangas <stefankangas@gmail.com> writes:

> branch: master
> commit f25f4699d760c5409a1571dcfdf4ed0fdac9e0ec
> Author: Stefan Kangas <stefankangas@gmail.com>
> Commit: Stefan Kangas <stefankangas@gmail.com>
>
>     Support several new DEs in wallpaper.el
>     
>     * lisp/image/wallpaper.el (wallpaper--default-commands)
>     (wallpaper--check-command, wallpaper-command): Add support for Unity,
>     Pantheon, Budgie, LXQt, and LXDE.  Also add support for Lubuntu,
>     Xubuntu, and Pop!_OS.
> ---
>  lisp/image/wallpaper.el | 43 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/image/wallpaper.el b/lisp/image/wallpaper.el
> index ff47d37e3a..effa25b180 100644
> --- a/lisp/image/wallpaper.el
> +++ b/lisp/image/wallpaper.el
> @@ -62,6 +62,16 @@
>      ;; XFCE
>      ("xfconf-query" "-c" "xfce4-desktop" "-p"
>       "/backdrop/screen0/monitoreDP/workspace0/last-image" "-s" "%f")
> +    ;; LXDE
> +    ("pcmanfm" "--set-wallpaper=%f")
> +    ;; LXQt
> +    ("pcmanfm-qt" "--set-wallpaper=%f") ; "--wallpaper-mode=MODE"
> +    ;; ;; Mate
> +    ;; ("gsettings" "set" "org.mate.background" "picture-filename" "%f")
> +    ;; ;; Cinnamon
> +    ;; ("gsettings" "set" "org.cinnamon.desktop.background" "picture-uri" "file://%f")
> +    ;; ;; Deepin
> +    ;; ("gsettings" "set" "com.deepin.wrap.gnome.desktop.background" "picture-uri" "file://%f")

BTW, did you test the file://%f commands with file names containing
spaces and/or Unicode to make sure they indeed do not have to be
encoded?

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 23 Sep 2022 12:52:10 +0800
> 
> BTW, did you test the file://%f commands with file names containing
> spaces and/or Unicode to make sure they indeed do not have to be
> encoded?

What kind of encoding did you have in mind in this case? IDNA?
url-hexlify? something else?

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> What kind of encoding did you have in mind in this case? IDNA?
> url-hexlify? something else?

url-hexify-string, which is what I think most GNOME-based desktops want.

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> BTW, did you test the file://%f commands with file names containing
> spaces and/or Unicode to make sure they indeed do not have to be
> encoded?

I did not test that, no.  For now, this is more based on searching for
recipes online rather than testing, unfortunately.

Do you have reason to think that they would need to be encoded?  My
findings so far would suggest that either they don't, or that a lot of
people are getting this wrong.

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Po Lu]

Stefan Kangas <stefankangas@gmail.com> writes:

> I did not test that, no.  For now, this is more based on searching for
> recipes online rather than testing, unfortunately.
>
> Do you have reason to think that they would need to be encoded?  My
> findings so far would suggest that either they don't, or that a lot of
> people are getting this wrong.

Only that they are encoded on my system running GNOME Shell:

  file:///usr/local/share/backgrounds/Simple%20white.png

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> Only that they are encoded on my system running GNOME Shell:
>
>   file:///usr/local/share/backgrounds/Simple%20white.png

OK, how about something like this (100 % untested)?

diff --git a/lisp/image/wallpaper.el b/lisp/image/wallpaper.el
index aea13227a8..0f4fc8b133 100644
--- a/lisp/image/wallpaper.el
+++ b/lisp/image/wallpaper.el
@@ -56,7 +56,7 @@ wallpaper--default-commands
   ;; When updating this, also update the custom :type for `wallpaper-command'.
   '(
     ;; Gnome
-    ("gsettings" "set" "org.gnome.desktop.background" "picture-uri"
"file://%f")
+    ("gsettings" "set" "org.gnome.desktop.background" "picture-uri"
"file://%F")
     ;; KDE Plasma
     ("plasma-apply-wallpaperimage" "%f")
     ;; XFCE
@@ -69,9 +69,9 @@ wallpaper--default-commands
     ;; ;; Mate
     ;; ("gsettings" "set" "org.mate.background" "picture-filename" "%f")
     ;; ;; Cinnamon
-    ;; ("gsettings" "set" "org.cinnamon.desktop.background"
"picture-uri" "file://%f")
+    ;; ("gsettings" "set" "org.cinnamon.desktop.background"
"picture-uri" "file://%F")
     ;; ;; Deepin
-    ;; ("gsettings" "set" "com.deepin.wrap.gnome.desktop.background"
"picture-uri" "file://%f")
+    ;; ("gsettings" "set" "com.deepin.wrap.gnome.desktop.background"
"picture-uri" "file://%F")
     ;; Sway (Wayland)
     ("swaybg" "-o" "*" "-i" "%f" "-m" "fill")
     ;; Wayland General
@@ -310,6 +310,9 @@ wallpaper-set
          (unless wallpaper-command
            (error "Couldn't find a command to set the wallpaper with"))
          (let* ((fmt-spec `((?f . ,(expand-file-name file))
+                            (?F . ,(mapconcat #'url-hexify-string
+                                              (split-string file "/")
+                                              "/"))
                             (?h . ,(wallpaper--get-height-or-width
                                     "height"
                                     #'display-pixel-height

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Yuri Khan]

On Fri, 23 Sept 2022 at 18:18, Stefan Kangas <stefankangas@gmail.com> wrote:

> Do you have reason to think that they would need to be encoded?  My
> findings so far would suggest that either they don't, or that a lot of
> people are getting this wrong.

Is URI syntax not reason enough? Unencoded spaces are not valid in
URIs, nor are non-ASCII characters, and various punctuation characters
that can appear in file names such as ‘?’ and ‘#’ are special in URIs.

Re: master f25f4699d7 2/2: Support several new DEs in wallpaper.el

[Stefan Kangas]

Stefan Kangas <stefankangas@gmail.com> writes:

> Po Lu <luangruo@yahoo.com> writes:
>
>> Only that they are encoded on my system running GNOME Shell:
>>
>>   file:///usr/local/share/backgrounds/Simple%20white.png
>
> OK, how about something like this (100 % untested)?

Now pushed to master as commit 3502fd9831.

> diff --git a/lisp/image/wallpaper.el b/lisp/image/wallpaper.el
> index aea13227a8..0f4fc8b133 100644
> --- a/lisp/image/wallpaper.el
> +++ b/lisp/image/wallpaper.el
> @@ -56,7 +56,7 @@ wallpaper--default-commands
>    ;; When updating this, also update the custom :type for `wallpaper-command'.
>    '(
>      ;; Gnome
> -    ("gsettings" "set" "org.gnome.desktop.background" "picture-uri" "file://%f")
> +    ("gsettings" "set" "org.gnome.desktop.background" "picture-uri" "file://%F")
>      ;; KDE Plasma
>      ("plasma-apply-wallpaperimage" "%f")
>      ;; XFCE
> @@ -69,9 +69,9 @@ wallpaper--default-commands
>      ;; ;; Mate
>      ;; ("gsettings" "set" "org.mate.background" "picture-filename" "%f")
>      ;; ;; Cinnamon
> -    ;; ("gsettings" "set" "org.cinnamon.desktop.background" "picture-uri" "file://%f")
> +    ;; ("gsettings" "set" "org.cinnamon.desktop.background" "picture-uri" "file://%F")
>      ;; ;; Deepin
> -    ;; ("gsettings" "set" "com.deepin.wrap.gnome.desktop.background" "picture-uri" "file://%f")
> +    ;; ("gsettings" "set" "com.deepin.wrap.gnome.desktop.background" "picture-uri" "file://%F")
>      ;; Sway (Wayland)
>      ("swaybg" "-o" "*" "-i" "%f" "-m" "fill")
>      ;; Wayland General
> @@ -310,6 +310,9 @@ wallpaper-set
>           (unless wallpaper-command
>             (error "Couldn't find a command to set the wallpaper with"))
>           (let* ((fmt-spec `((?f . ,(expand-file-name file))
> +                            (?F . ,(mapconcat #'url-hexify-string
> +                                              (split-string file "/")
> +                                              "/"))
>                              (?h . ,(wallpaper--get-height-or-width
>                                      "height"
>                                      #'display-pixel-height