[rfc][icedtea-web] fixinf of desktop icon behaviour
Adam Domurad
adomurad at redhat.com
Fri Dec 28 09:15:36 PST 2012
On 12/24/2012 03:51 PM, Jiri Vanek wrote:
> On 12/24/2012 03:06 PM, Jiri Vanek wrote:
>> On 12/21/2012 07:55 PM, Jiri Vanek wrote:
>>> On 12/21/2012 06:04 PM, Xerxes Rånby wrote:
>>>> 2012-12-21 17:05, Jiri Vanek skrev:
>>>>> Hi! This is fix for
>>>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=725 - JNLP
>>>>> applications will prompt for creating desktop shortcuts every time
>>>>> they are run
>>>>>
>>>>> When I have seen Omairs original patch it was too much
>>>>> complicated, So I want at least to try this one.
>>>>> It is pretty simple. The disadvantage however is:
>>>>> + public File getFinalLinuxDesktopIconFile() {
>>>>> + return new
>>>>> File(System.getProperty("user.home")+"/Desktop/"+getDesktopIconFinalName()+".desktop");
>>>>> + }
>>>>>
>>>>> to be dependent on System.getProperty("user.home")+"/Desktop/" and
>>>>> .desktop suffix.
>>>>>
>>>>> However .. can it be enough?
>>>>>
>>>>> The second patch is for testing purposes of this case which I
>>>>> would like to push forward as soon as possible.
>>>>>
>>>>> J.
>>>>
>>>> This will be broken on non-english systems.
>>>>
>>>> The name of the Desktop folder gets localized on many distributions.
>>>> For example the Desktop folder is called Skrivbord on Swedish user
>>>> systems.
>>>> The configuration of the user desktop folder location is set in the
>>>> ~/.config/user-dirs.dirs
>>>>
>>>> On my system this file contains:
>>>> xranby at xranby-ESPRIMO-P7935:~/.config$ cat user-dirs.dirs
>>>> # This file is written by xdg-user-dirs-update
>>>> # If you want to change or add directories, just edit the line you're
>>>> # interested in. All local changes will be retained on the next run
>>>> # Format is XDG_xxx_DIR="$HOME/yyy", where yyy is a shell-escaped
>>>> # homedir-relative path, or XDG_xxx_DIR="/yyy", where /yyy is an
>>>> # absolute path. No other format is supported.
>>>> #
>>>> XDG_DESKTOP_DIR="$HOME/Skrivbord"
>>>> XDG_DOWNLOAD_DIR="$HOME/Hämtningar"
>>>> XDG_TEMPLATES_DIR="$HOME/Mallar"
>>>> XDG_PUBLICSHARE_DIR="$HOME/Publikt"
>>>> XDG_DOCUMENTS_DIR="$HOME/Dokument"
>>>> XDG_MUSIC_DIR="$HOME/Musik"
>>>> XDG_PICTURES_DIR="$HOME/Bilder"
>>>> XDG_VIDEOS_DIR="$HOME/Video"
>>>>
>>>
>>> I see.. This is very valid point and I have hoped to hear something
>>> like this.
>>> Do you think it is correct to get this information from
>>> ~/.config/user-dirs.dirs ?
>>> It sound good enough to me to parse it... As it is standartized in
>>> freedesktop.org.
>>>
>>> Thanx for reply,
>>>
>>> J.
>>
>> So here is improved version which is parsing fro
>> ~/.config/user-dirs.dirs.
>>
>> J.
>
>
> This is little bit better and is taking care for all possible
> variables. Instead all this parsing and substitutions simple exec of
> echo $(xdg-user-dir DESKTOP) is possible. And it maybe more bullet
> proof. However I would like avoid Sytem.exec wheres possible.
>
> J.
>
thanks, comments inline
> diff -r a16da8b96a0f
> netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java
> --- a/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Mon
> Dec 24 13:58:31 2012 +0100
> +++ b/netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java Mon
> Dec 24 21:38:14 2012 +0100
> @@ -146,7 +146,13 @@
> private void addMenuAndDesktopEntries() {
> XDesktopEntry entry = new XDesktopEntry(file);
> ShortcutDesc sd = file.getInformation().getShortcut();
> -
> + if (entry.getFinalLinuxDesktopIconFile().exists()) {
> + if (JNLPRuntime.isDebug()) {
> + System.out.println("ApplicationInstance.addMenuAndDesktopEntries():
> file - "
> + +
> entry.getFinalLinuxDesktopIconFile().getAbsolutePath() + " already
> exists. Not proceeding with desktop additions");
> + }
> + return;
> + }
> if (shouldCreateShortcut(sd)) {
> entry.createDesktopShortcut();
> }
> diff -r a16da8b96a0f netx/net/sourceforge/jnlp/util/XDesktopEntry.java
> --- a/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Mon Dec 24
> 13:58:31 2012 +0100
> +++ b/netx/net/sourceforge/jnlp/util/XDesktopEntry.java Mon Dec 24
> 21:38:14 2012 +0100
> @@ -16,16 +16,24 @@
>
> package net.sourceforge.jnlp.util;
>
> +import java.io.BufferedReader;
> import java.io.File;
> import java.io.FileNotFoundException;
> import java.io.FileOutputStream;
> +import java.io.FileReader;
> import java.io.IOException;
> import java.io.OutputStreamWriter;
> import java.io.Reader;
> import java.io.StringReader;
> import java.net.URL;
> import java.nio.charset.Charset;
> +import java.util.ArrayList;
> import java.util.Arrays;
> +import java.util.Collections;
> +import java.util.Comparator;
> +import java.util.List;
> +import java.util.Map.Entry;
> +import java.util.Set;
>
> import net.sourceforge.jnlp.IconDesc;
> import net.sourceforge.jnlp.JNLPFile;
> @@ -48,6 +56,7 @@
>
> public static final String JAVA_ICON_NAME = "java";
>
> +
> private JNLPFile file = null;
> private int iconSize = -1;
> private String iconLocation = null;
> @@ -78,7 +87,7 @@
>
> String fileContents = "[Desktop Entry]\n";
> fileContents += "Version=1.0\n";
> - fileContents += "Name=" + sanitize(file.getTitle()) + "\n";
> + fileContents += "Name=" + getDesktopIconFinalName() + "\n";
> fileContents += "GenericName=Java Web Start Application\n";
> fileContents += "Comment=" +
> sanitize(file.getInformation().getDescription()) + "\n";
> fileContents += "Type=Application\n";
> @@ -122,6 +131,11 @@
> return iconSize;
> }
>
> + public File getShortcutTmpFile() {
> + File shortcutFile = new
> File(JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_USER_TMP_DIR)
> + File.separator + FileUtils.sanitizeFileName(file.getTitle()) +
> ".desktop");
bit of a mouthful, a temporary variable here for the user directory
would be good.
> + return shortcutFile;
> + }
> +
> /**
> * Set the icon size to use for the desktop shortcut
> *
> @@ -148,9 +162,7 @@
> * Install this XDesktopEntry into the user's desktop as a launcher
> */
> private void installDesktopLauncher() {
> - File shortcutFile = new File(JNLPRuntime.getConfiguration()
> - .getProperty(DeploymentConfiguration.KEY_USER_TMP_DIR)
> - + File.separator +
> FileUtils.sanitizeFileName(file.getTitle()) + ".desktop");
> + File shortcutFile = getShortcutTmpFile();
> try {
>
> if (!shortcutFile.getParentFile().isDirectory() &&
> !shortcutFile.getParentFile().mkdirs()) {
> @@ -234,4 +246,80 @@
> }
> }
>
> + public String getDesktopIconFinalName() {
I don't understand what is 'final' about it ? Why can it simply not be
called the icon name?
> + return sanitize(file.getTitle());
> + }
> +
> + public File getFinalLinuxDesktopIconFile() {
Similarly I don't understand what makes this the final linux desktop
icon file, vs 'getLinuxDesktopIconFile'
> + return new File(findFreedesktopOrgDesktopPathCatch() +
> getDesktopIconFinalName() + ".desktop");
> + }
> +
> + public static String findFreedesktopOrgDesktopPathCatch() {
I'm all for splitting into many small methods, but I don't know how I
feel about findFreedesktopOrgDesktopPathCatch,
findFreedesktopOrgDesktopPath, getFreedesktopOrgDesktopPathFrom all
being public, separate methods. I think just inlining all the logic into
one findFreedesktopOrgDesktopPath() method suffices, also the name
'findFreedesktopOrgDesktopPathCatch' says too much about the
implementation IMO.
> + try {
> + return findFreedesktopOrgDesktopPath();
> + } catch (Exception ex) {
I think catching Exception is (almost) always a bad idea, it can silence
even array-out-of-bounds. I think IOException suffices.
> + return System.getProperty("user.home") + "/Desktop/";
> + }
> + }
> +
> + public static String findFreedesktopOrgDesktopPath() throws
> IOException {
> + File userDirs = new File(System.getProperty("user.home") +
> "/.config/user-dirs.dirs");
> + if (!userDirs.exists()) {
> + return System.getProperty("user.home") + "/Desktop/";
> + }
> + return getFreedesktopOrgDesktopPathFrom(userDirs);
> + }
> +
> + public static String getFreedesktopOrgDesktopPathFrom(File
> userDirs) throws IOException {
> + BufferedReader r = new BufferedReader(new FileReader(userDirs));
> + try {
> + return getFreedesktopOrgDesktopPathFrom(r);
> + } finally {
> + r.close();
> + }
> +
> + }
> +
> + public static final String XDG_DESKTOP_DIR = "XDG_DESKTOP_DIR";
> +
> + public static String
> getFreedesktopOrgDesktopPathFrom(BufferedReader r) throws IOException {
This is a helper method and shouldn't be public IMO.
> + while (true) {
> + String s = r.readLine();
> + if (s == null) {
> + throw new IOException("End of user-dirs found, but no
> " + XDG_DESKTOP_DIR + " key found");
> + }
> + s = s.trim();
> + if (s.startsWith(XDG_DESKTOP_DIR)) {
> + if (!s.contains("=")) {
> + throw new IOException(XDG_DESKTOP_DIR + " have no
> value");
s/have/has/
> + }
> + String[] ss = s.split("=");
rename 'ss' to 'keyvalue' or something, and the reason ss[1] is used
will be clearer IMO.
> + ss[1] = ss[1].trim();
> + return evaluateVariables(ss[1]);
> + }
> + }
> + }
> +
> + private static String evaluateVariables(String orig) {
> +
> + Set<Entry<String, String>> env = System.getenv().entrySet();
> + List<Entry<String, String>> envVariables = new
> ArrayList<Entry<String, String>> (env);
> + Collections.sort(envVariables, new Comparator<Entry<String,
> String>>(){
> +
> + @Override
> + public int compare(Entry<String, String> o1,
> Entry<String, String> o2) {
> + return o2.getKey().length()-o1.getKey().length();
> + }
> + });
> + while (true) {
> + String before = orig;
> + for (Entry<String, String> entry : envVariables) {
> + orig=orig.replaceAll("\\$"+entry.getKey(),
> entry.getValue());
> + }
> + if (before.equals(orig)) {
> + return orig;
> + }
> + }
> +
> + }
> }
> diff -r a16da8b96a0f
> tests/netx/unit/net/sourceforge/jnlp/util/XDesktopEntryTest.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/netx/unit/net/sourceforge/jnlp/util/XDesktopEntryTest.java
> Mon Dec 24 21:38:14 2012 +0100
> @@ -0,0 +1,59 @@
> +/*
> + * To change this template, choose Tools | Templates
> + * and open the template in the editor.
> + */
Netbeans user are you ? Anyway this should be gone.
> +package net.sourceforge.jnlp.util;
> +
> +import java.io.BufferedReader;
> +import java.io.IOException;
> +import java.io.StringReader;
> +import org.junit.Assert;
> +import org.junit.Test;
> +
> +public class XDesktopEntryTest {
> +
> + private String des1 = "/my/little/Desktop";
> + private String des2name = "Plocha";
> + private String des2Res = System.getProperty("user.home") + "/" +
> des2name;
> + private String des2 = "$HOME" + "/" + des2name;
> + private String src1 = XDesktopEntry.XDG_DESKTOP_DIR + "=" + des1;
> + private String src2 = " " + XDesktopEntry.XDG_DESKTOP_DIR + " =
> " + des1;
> + private String src3 = "#" + XDesktopEntry.XDG_DESKTOP_DIR + " = "
> + des1;
> + private String src4 = XDesktopEntry.XDG_DESKTOP_DIR + "=" + des2;
> + private String src5 = " " + XDesktopEntry.XDG_DESKTOP_DIR + " =
> " + des2;
> + private String src6 = "#" + XDesktopEntry.XDG_DESKTOP_DIR + " = "
> + des2;
> +
> + @Test
> + public void getFreedesktopOrgDesktopPathFromtestSimple() throws
> IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src1)));
> + Assert.assertEquals(s, des1);
> + }
> +
> + @Test
> + public void getFreedesktopOrgDesktopPathFromtestSpaced() throws
> IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src2)));
> + Assert.assertEquals(s, des1);
> + }
> +
> + @Test(expected = IOException.class)
> + public void getFreedesktopOrgDesktopPathFromtestCommented()
> throws IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src3)));
> + }
> +
> + @Test
> + public void getFreedesktopOrgDesktopPathFromtestSimpleWithHome()
> throws IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src4)));
> + Assert.assertEquals(s, des2Res);
> + }
> +
> + @Test
> + public void getFreedesktopOrgDesktopPathFromtestSpacedWithHome()
> throws IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src5)));
> + Assert.assertEquals(s, des2Res);
> + }
> +
> + @Test(expected = IOException.class)
> + public void
> getFreedesktopOrgDesktopPathFromtestCommentedWithHome() throws
> IOException {
> + String s = XDesktopEntry.getFreedesktopOrgDesktopPathFrom(new
> BufferedReader(new StringReader(src6)));
> + }
> +}
Test looks OK
Just one additional comment, as Omair pointed out his patch had nice
functionality when a desktop icon was deleted (it did not prompt again
for desktop icon to be created). It is worth considering.
Happy hacking,
-Adam
More information about the distro-pkg-dev
mailing list