Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extraneous double-quotes in dmenuExtended_aliases_lookup.json #108

Open
galou opened this issue Jun 3, 2020 · 13 comments
Open

Extraneous double-quotes in dmenuExtended_aliases_lookup.json #108

galou opened this issue Jun 3, 2020 · 13 comments

Comments

@galou
Copy link

galou commented Jun 3, 2020

I have a .desktop file with Exec="/home/gael/.cache/cesnet-owncloud/03-tmp/AppImage/PrusaSlicer-2.2.0+linux-x64-202003211856.AppImage" %U which dmenu-extended converts to

    [
        "PrusaSlicer AppImage",
        "\"/home/gael/.cache/cesnet-owncloud/03-tmp/AppImage/PrusaSlicer-2.2.0+linux-x64-202003211856.AppImage\""
    ],

These extra double-quotes prevent the menu entry to work. Removing them fixes the issue but this could happen that the double-quotes are required in the .desktop file.

Otherwise, I like dmenu-extended very much.

@Fuseteam
Copy link

Fuseteam commented Jun 3, 2020

do you really require the double quotes in your desktop file?

@MarkHedleyJones
Copy link
Owner

Thanks for the feedback :)

  1. Quotes for the Exec entry in the .desktop file will be required since there may be spaces in the path.
  2. Double quotes shouldn't be stored, so there needs to be some detection of already quoted Exec statements to fix this issue.
  3. Thanks for your feedback - I'm glad the extension useful for you.

I can't comment on when I'll get time to fix this, but please feel free to take a look if you have time.
Ideally, there would be a test to detect this issue too.

@galou
Copy link
Author

galou commented Jun 4, 2020

Thanks for the quick reaction! For sure I could have a look at it but I'd like to have a hint where I should start to render things quicker.

@MarkHedleyJones
Copy link
Owner

No problem.
I would start here:

for line in f.readlines():
if line[0:5] == 'Exec=' and command is None:
command_tmp = line[5:-1].split()
command = ''
space = ''
for piece in command_tmp:
if piece.find('%') == -1:
command += space + piece
space = ' '
else:
break

This part is responsible for extracting the command from .desktop files.
It's not very pretty :(

@galou
Copy link
Author

galou commented Jun 4, 2020

I just have a small patch for now:

diff --git a/dmenu_extended/main.py b/dmenu_extended/main.py
index 4fc41e5..10f564e 100755
--- a/dmenu_extended/main.py
+++ b/dmenu_extended/main.py
@@ -863,9 +863,18 @@ class dmenu(object):
                         terminal = None
                         for line in f.readlines():
                             if line[0:5] == 'Exec=' and command is None:
-                                command_tmp = line[5:-1].split()
                                 command = ''
-                                space = ''
+                                if line[5:-1].strip().startswith('"'):
+                                    from_ = line.find('"')
+                                    to = line[(from_ + 1):].find('"') + from_
+                                    if to == -1:
+                                        if self.debug:
+                                            print('Malformed Exec=')
+                                        continue
+                                    command = line[(from_ + 1):(to + 1)].replace(' ', '\\ ')
+                                    line = line[:5] + line[(to + 2):-1].strip()
+                                command_tmp = line[5:-1].strip().split()
+                                space = ' ' if command else ''
                                 for piece in command_tmp:
                                     if piece.find('%') == -1:
                                         command += space + piece

I don't know what to do with spaces within command (I replaced with "\ " for now) and I didn't write any test. I also don't handle other double-quotes than the first pair because I don't know what to do with them.

@galou
Copy link
Author

galou commented Jun 4, 2020

I have another more elegant version with shlex.split:

diff --git a/dmenu_extended/main.py b/dmenu_extended/main.py
index 4fc41e5..a4ff874 100755
--- a/dmenu_extended/main.py
+++ b/dmenu_extended/main.py
@@ -10,6 +10,7 @@ import codecs
 import locale
 import operator
 import time
+import shlex
 
 Help = """
 Dmenu Extended command line options
@@ -863,15 +864,13 @@ class dmenu(object):
                         terminal = None
                         for line in f.readlines():
                             if line[0:5] == 'Exec=' and command is None:
-                                command_tmp = line[5:-1].split()
-                                command = ''
-                                space = ''
-                                for piece in command_tmp:
-                                    if piece.find('%') == -1:
-                                        command += space + piece
-                                        space = ' '
-                                    else:
-                                        break
+                                splits = shlex.split(line[5:])
+                                has_percent = [s.startswith('%') for s in splits]
+                                if any(has_percent):
+                                    index_percent = has_percent.index(True) + 1
+                                else:
+                                    index_percent = len(splits)
+                                command = ' '.join(s.replace(' ', '\\ ') for s in splits[0:index_percent])
                             elif line[0:5] == 'Name=' and name is None:
                                 name = line[5:-1]
                             elif line[0:12] == 'GenericName=' and name_generic is None:

@Fuseteam
Copy link

Fuseteam commented Jun 4, 2020

i meant to ask @galou this :p as if i read correctly the exec line in your desktop file may not require the double quotes at all to work

@MarkHedleyJones
Copy link
Owner

@Fuseteam, just to be clear... by double quotes are you referring to "\"... or just "...
In @galou's .desktop file I'm imagining Exec="/home/gael/.cache...
Now, since there's an absolute path to the executable (because the executable's isn't in the @galou's PATH) and that path may contain spaces, it seems appropriate to be quoted.

This is an interesting problem. I think dmenu-extended should work with quoted Exec statements.
What if the AppImage name (or path) contained a space? It would have to be quoted to delimit the path.

I agreee though, things would be much simpler if the quotes were just removed from the .desktop file.
But, if quoted content can be in there, then dmenu-extended should be able to handle it.

@Fuseteam
Copy link

Fuseteam commented Jun 5, 2020

@MarkHedleyJones ah yes i agree with you, i was just pointing out the double may not be neccesary in @galou's case
ofcourse if dmenu-extended can just handle it that would be the best either way

@galou
Copy link
Author

galou commented Jun 8, 2020

I wrote the code that handles the double-quotes in the .desktop file but I don't know how to handle spaces in the command variable in the code of dmenu-extended. I'll finish my patch when I get this information.

@MarkHedleyJones
Copy link
Owner

@galou sorry for the delay!
Are you able to open a pull request for your code?
Also, I'm not sure exactly what you mean by handling spaces in the command variable.
Do you mean, it's failing when command contains spaces?
It would be good to have some test cases for this.

@galou
Copy link
Author

galou commented Jun 23, 2020

By "handling spaces in the command variable" I mean "do the spaces have to be escaped and if yes how?"

@galou
Copy link
Author

galou commented Jun 24, 2020

I made an error in my script: index_percent = has_percent.index(True) + 1 should be replaced with index_percent = has_percent.index(True).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants