bunsenlabs / bunsen-pipemenus Goto Github PK

All pipemenus failing to execute command on startup because directories do not exist anymore.

The "-e" (execute) option argument is supported across the range of common programs used for the x-terminal-emulator, but the invocation is not uniform. It may be quoted or unquoted:

x-terminal-emulator -e "man tint2"              
x-terminal-emulator -e man tint2                

Some emulators require one form or the other, with no common denominator:

                Only            Only                            Unquoted
                quoted          unquoted        Either          using "-x"
xterm                                           X               NA
urxvt                           X                               NA
xfce4-terminal  X                                               X
lxterminal                                      X               NA
roxterm                         X                               NA
terminator      X                                               X

Note that all emulators requiring quotes with "-e" also accept unquoted versions using "-x" in place of "-e" (as noted). The pipemenus need to set correct -e/-x (or quoted/unquoted) commands to accommodate all x-terminal-emulator alternatives.

When running a new install, the command in ~/.config/openbox/autostart to launch tint2 is (sleep 2s && bl-tint2-session) &
If there is no session file, then bl-tint2-session runs the command tint2 with no config file defined. tint2 will then load the default ~/.config/tint2/tint2rc.

Now, on the menu Settings > Tint2>Edit Tint2s > Running Tint2s offers to edit the file tint2/tint2
Clicking this opens the non-existent file ~/tint2

The problem comes from loadTEditmenu() in bl-tint2-pipemenu:

loadTEditmenu(){
    menuSeparator 
    menuItem "Default tint2rc" "bl-text-editor $TINT2RC"
    if [ "$(pidof tint2)" ];then
        menuSubmenu "RunningTint2" "Running Tint2s"
        pgrep -a tint2 | while read pid cmd; do 
            if [[ ${cmd%% *} = tint2 ]]; then
                TINT2=$(echo $cmd | awk -F"/" '{print $(NF-1)"/"$NF}')
                TINT=$(echo $cmd | awk '{print $NF}')
                menuItem "$TINT2" "bl-text-editor $TINT"
            fi
        done
        menuSubmenuEnd
    fi
}

pgrep -a tint2 returns:

7135 /bin/bash /usr/bin/bl-tint2-session
7228 tint2

The awk lines above are returning tint2/tint2 and tint2 respectively, while what is desired in this case is something like default tint2rc' and~/.config/tint2/tint2rc`.

SUGGEST: either amend loadTEditmenu() to set the default tint2rc when no config file is in the command line, or perhaps ship a default tint2 session file (although the problem with loadTEditmenu() would remain in that case).

Code snippet from my ~/.local/share/recently-used.xbel:

  <bookmark href="file:///home/john/Documents/Band%20Stuff/photos/funky-archive_o.zip" added="2016-07-31T05:34:06Z" modified="2016-07-31T05:34:17Z" visited="2016-07-31T05:34:06Z">
    <info>
      <metadata owner="http://freedesktop.org">
        <mime:mime-type type="application/zip"/>
        <bookmark:applications>
          <bookmark:application name="File Roller" exec="&apos;file-roller&apos;" modified="2016-07-31T05:34:17Z" count="2"/>
        </bookmark:applications>
      </metadata>
    </info>
  </bookmark>

Clicking the corresponding item in the menu, "funky-archive_o.zip" brings up an openbox error box with the content:

Failed to execute child process "file-rol" (No such file or directory)

For some reason the last three characters are removed. Other menu items open successfully.

The only difference I can see in recently-used.xbel is that other applications (including bl-image-viewer) write their exec attributes as 'command %u' and file-roller omits the ' %u'. I think the script should be able to handle that.

Because Lua 5.3 has become the new default Lua implementation (although 5.2 seems to stay available).

Ref bl- alternatives, lines 32/33:

# Default command to open files with - others might be xdg-open, gnome-open, thunar, #pcmanfm...
default_open_cmd=xdg-open # exo-open comes with thunar

Keep xdg-open I guess?

...and should those commands be quoted? eg
text_editor='bl-text-editor'

bl-x-www-browser-pipemenu will attempt to install "firefox" if called from the menu, which will fail because Debian Jessie offers only "firefox-esr". (Since firefox is installed by default this will affect only a few users.)

Fix:

1. Edit ~/.config/openbox/pipemenus.rc to refer to 'firefox-esr', and edit bl-x-www-browser-pipemenu to display the app name as "Firefox" not "Firefox Esr".

or

2. Edit bl-x-www-browser-pipemenu to install 'firefox-esr' when asked to install 'firefox'.

(2) requires editing one file only, but (1) keeps the integrity of the package list in pipemenus.rc as referring to packages that exist!

Opinions?

bl-kb-pipemenu is a little slow in execution (~270ms on my system) which makes the openbox menu "catch" when the cursor passes that point.
It can be made faster by making the "Display Keybinds in menu" submenu a new pipemenu, executed by bl-kb-pipemenu --keybinds.
Another small optimization is only to call bl-kb when it is needed in that submenu.
(I have also made the script exit after editing rc.xml to avoid unnecessary output to stdout.)

I have put these changes in the kb branch of this repository. Although the submenu takes as long to open as before, the outside menu now executes in 17ms and the overall feel is much smoother.

If this is acceptable I will merge kb into master.

While looking into this I found a small modification is possible to bl-kb to avoid output to stout with the --gui option. I will post that in bunsen-utilities.
BunsenLabs/bunsen-utilities#42

It seems like the format returned by paramiko has changed since the conception of this script (ssh-pipemenu). I have prepared a patch. Would like to submit it here since there are certain changes that I wish to maintain in mine that might not suit the standard BL config

diff --git a/bl-sshconfig-pipemenu b/bl-sshconfig-pipemenu
index 24b4b4b..ed16a57 100755
--- a/bl-sshconfig-pipemenu
+++ b/bl-sshconfig-pipemenu
@@ -1,6 +1,7 @@
 #!/usr/bin/env python

-import os, warnings
+import os
+import warnings
 with warnings.catch_warnings():
     warnings.simplefilter("ignore")
     from paramiko.config import SSHConfig
@@ -12,8 +13,8 @@ try:
     config_file = file(cfgfile)
 except IOError:
     if not os.path.exists(cfgdir):
-            os.makedirs(cfgdir,0700)
-    f = open(cfgfile,'w')
+        os.makedirs(cfgdir, 0700)
+    f = open(cfgfile, 'w')
     o = '# SSH config file, \'man ssh_config\' for more details.\n\n'
     o += '#Host example\n'
     o += '#    hostname example.com\n'
@@ -34,26 +35,27 @@ print '<openbox_pipe_menu>\n'

 if len(hosts) >= 2:
     for h in hosts[1:]:
-        if h.has_key('host') and h.has_key('hostname'):
+        if 'host' in h and 'hostname' in h['config']:
+            conf = h['config']
             user = ''
-            if h.has_key('user'):
-                user = '-l '+h['user']+ ' '
-            port = ['','']
-            if h.has_key('port'):
-                port[0] = '-p '+h['port']+ ' '
-                port[1] = ':'+h['port']
-            print '<menu id="ssh-'+h['host']+'" label="'+h['host']+'">'
+            if 'user' in conf:
+                user = '-l ' + conf['user'] + ' '
+            port = ['', '']
+            if 'port' in conf:
+                port[0] = '-p ' + conf['port'] + ' '
+                port[1] = ':' + conf['port']
+            print '<menu id="ssh-'+h['host'][0]+'" label="'+h['host'][0]+'">'
             print '    <item label="Start terminal session">'
             print '        <action name="Execute">'
             print '            <command>'
-            print '                x-terminal-emulator -T ssh '+user+port[0]+h['hostname']
+            print '                x-terminal-emulator --command=&quot;ssh ' + user + port[0] + conf['hostname'] + '&quot;'
             print '            </command>'
             print '        </action>'
             print '    </item>\n'
-            print '    <item label="Browse with File Manager">'
+            print '    <item label="Browse with Thunar">'
             print '        <action name="Execute">'
             print '            <command>'
-            print '                bl-file-manager ssh://'+h['hostname']+port[1]
+            print '                thunar ssh://' + conf['hostname'] + port[1]
             print '            </command>'
             print '        </action>'
             print '    </item>\n'
@@ -63,11 +65,9 @@ if len(hosts) >= 2:
 print '<item label="Edit ~/.ssh/config">'
 print '    <action name="Execute">'
 print '        <command>'
-print '            bl-text-editor ~/.ssh/config'
+print '            run_vim ~/.ssh/config'
 print '        </command>'
 print '    </action>'
 print '</item>\n'

 print '</openbox_pipe_menu>'
-
-

...for the Opera and google-chrome apt-keys

There will be a symlink from ~/.conkyrc to wherever the default conky really is, but mightn't it be better if scripts all refer to the real location?
ie change CONKYDEFAULT="$HOME/.conkyrc" to something like CONKYDEFAULT="$CONKYPATH/default" ? Looks a bit neater, and allows CONKYPATH to be changed more easily.

In fact, it might be possible for the pipemenu to check for the existence of that symlink at ~/.conkyrc and make it if necessary, and if there's already a conky config file there, to move it to the new location?

The pipemenu installers invoke the terminal emulator with a title for a professional appearance. The scripts use option argument "--title". While supported by the common GTK compliant terminal emulators (xfce4-terminal, roxterm, lxterminal, terminator), this option is not supported by xterm
and rxvt/urxvt. Fortunately, all of these emulators support the legacy "-T" option originating with xterm. As the x-terminal-emulator can be mapped to any one of these alternatives, "-T" not "--title" should be the standard option for specifying a title.

When installing an application from a pipemenu, the command "Hit any key to exit..." does not close the screen.

...if only one browser is installed.

When running the command in a terminal:

sudo update-alternatives --config x-www-browser
[sudo] password for user: 
There is only one alternative in link group x-www-browser (providing /usr/bin/x-www-browser): /usr/bin/firefox-esr
Nothing to configure.

If run from the menu item, the screen just exits without informing the user there is nothing to configure. So causing confusion to a new user, who may think there is a bug! https://forums.bunsenlabs.org/viewtopic.php?id=2867

The graphics and browser pipemenus both use an app/browserExists() function, and several other pipemenus I am working on also use this. There are also the sections involving setupFunctionName and promptInstall which are duplicated many times.

Is it worth making functions in bl-include.cfg to save multiple instances of the same code? It would be more elegant, but is it worth the effort?

Do pipemenus need an "install" entry in every case for apps not on the user's system? If it's something (s)he has no intention of installing it will clutter up the menu unnecessarily.

Maybe we could give some thought to which cases justify an "install" and which can just be omitted from the menu if they're not installed? (Of course those entries would appear after the app was installed.)

eg Add "install" option if the installation is more complicated than a simple apt-get, or if it's something we are recommending the user to install...?

Also true for RC2 image provided by has.

Noticed this while looking at the media players thing. Is there any reason to have an entry in the multimedia menu to install or invoke a systray volume icon? Now volti's being replaced, shall I just take it out of the menu?

This is already done in many places in the pipemenus, but if all calls to scripts in bunsen-utilities were preceded by a test before adding the menu item, like this in bl-tint2-pipemenu:

loadRestartmenu(){
    if [ "$(pidof tint2)" ];then
        if type bl-tint2restart &>/dev/null;then  # bl-tint2restart script found
            menuItem "Restart Tint2" "bl-tint2restart"
        fi
    fi
}

then bunsen-utilities would not have to be a hard dependency of bunsen-pipemenus. A Recommends: would be enough, and the menu items needing those scripts would just be missing if the package wasn't installed.

For example, at present there is no such test for bl-tint2zen in the same pipemenu.

The terminal throws an error when I tried installing google chrome. Apparently, it's not adding the chrome repo in sources.list

bunsen-pipemenus has a long list of dependencies: https://github.com/BunsenLabs/bunsen-pipemenus/blob/master/debian/control
including, for example both wget and curl. (That looks like a case for rewriting one of the scripts to use, maybe, wget.)

I have already moved compton from Depends: to Recommends: as only the bl-compositor menu would be affected by its absence.

Maybe there are some other packages that could be recommends? We don't want to force people to install a lot of unnecessary packages.

In a fresh install from a Jessie netinstall and the bunsen-netinstall setup, I recieved this error while hovering over "Recent Files" in the OB root menu.

Invalid output from pipe-menu "bl-recent-files-pipemenu -rl15"

I opened then closed a file from the command line (vim) and tried it again. Same gtk popup with the same error. I opened then closed a file using thunar in geany and tried it again. The gtk popup was gone and the file I had opened was listed in the menu in the space under "Clear recent files".

The output from executing /usr/bin/bl-recent-files-pipemenu (without having a ~/.local/share/recently-used.xbel file) from the command line is as follows:

lua 5.2: /usr/bin/bl-recent-files-pipemenu:125: attempt to concatenate local 'path' (a nil value)
stack traceback
    /usr/bin/bl-recent-files-pipemenu:125: in function 'add_remove_file_item'
    /usr/bin/bl-recent-files-pipemenu:198: in function 'main'
    /usr/bin/bl-recent-files-pipemenu:213: in main chunk
    [C]: in ?

BUT after $ touch ~/.local/share/recently-used.xbel the output reacts as expected; aka not listing any recent files, but displaying only a single entry of "Clear recent files". I would assume that this file would need to be placed in the netinstall script in order for this functionality to occur OOTB.

As somewhat of a sidenote, when attempting to revert back to the state without any cached recent files I had selected "Clear recent files", it did nothing. The files still existed in the menu. This was repeated after opening several files via thunar/geany, and then closing them. There were then several files in the "Recent Files" pipemenu, and after hitting "Clear recent files" and rechecking, they were still there.

There was no error given, but the files were still listed as recent files.

@johnraff

Now more consistent - edits to bl-help-pipemenu and bl-conky-pipemenu

The bl-help-pipemenu (pending pull request #14) invokes man pages using the x-terminal-emulator but does not supply titles like the pipemenu installers. A title matching the menu name would be more consistent and polished.

If python-gpgme is needed during Dropbox install, there is nothing to show any progress, so it seems that the script has stopped. Users may assume the install has failed, if they don't wait long enough.

Proposed fix: change

gksu 'apt-get install -y python-gpgme' || {

to

x-terminal-emulator -T 'Install python-gpgme' -e sudo apt-get install -y python-gpgme || {

@johnraff @2ion

Get running tint2s with pgrep;
don't show Edit menu items if tint2s aren't runnng;
restart Tint2s with bl-tint2restart

We need a test for these commandline parameters, which may be a common action. ATM they cause the openbox menu to be output.

I suggest that it should display a link to the config file, $HOME/.config/compton.conf.

This is a continuation to this discussion:
f752c5c#commitcomment-10089365

Although the answer was "no", I thought that I should open an issue just for a bit of peer review. Feel free to close it a tad later.

Sure, they're not the same in compound cases. I think most people understand this.

If only this was true.

Reasons to use && {:

• if you want to turn Bash into C-like language where { } are used for blocks.
• ... && { ... } is 6 characters shorter than if ... ; then ... fi

Reasons to use traditional ifs:

• It is safe. See Bash Pitfall #22
• Consistency. if ...; then everywhere, there is no need to have different syntax for conditionals with else clause.
• You don't have to change the syntax if you just want to add else
• Less symbol clutter. [[ ... ]] && { is less readable than if [[ ... ]]; then
• Conventions. Traditional ifs are much more popular. (I wish I could seach for && { on github, but unfortunately most search engines do not allow special symbols in search queries).

In the multimedia menu the entry for smplayer is changed to read "Gnome mplayer", while there is another different package gnome-mplayer.

Change that entry back to Smplayer and add gnome-mplayer (also a reasonable alternative) to skel/.config/openbox/pipemenus.rc in bunsen-configs?

I noticed John was using "#!/bin/sh" in scripts "in the hope that dash will be used".

Should all the pipemenus use this?