Created attachment 4540 Patch for enhancement * Created option to allow a command to be specified * When specified, command is executed and the (first line of the) output is used as the network device * Toggling run-command option on disables the network-device field and vice-versa.
Hi Miguel, First to answer some of your comments in the patch. /* enough? */ => That's up to you, you provide the patch. /* What is the XFCE way of dealing with null pointers */ => There is no Xfce way, it's just C code. Second about know the purpose of this patch. The sanitizing part will only work for commands that return exactly what is expected. The sanitizing part fails in case the output doesn't end with a new line (\n). Can you cite a command that works for this patch? I don't see ip nor ipconfig as an example. This may mean that the user needs to craft his own script, which defeats the purpose of an option. As a user it's impossible to guess what this new option is about. Finally it's merely a duplicate of bug 3880, or bug 7431. Your way of trying to discover the interface is not good enough, and is therefore rejected. Thank you for your report. Regards, Mike
Hi Mike, >> /* What is the XFCE way of dealing with null pointers */ > => There is no Xfce way, it's just C code. Understood. I'm new to XFCE (and Open Source) development and just didn't know whether you guys had your own guidelines/standards. > Second about know the purpose of this patch. > > The sanitizing part will only work for commands that return exactly what is > expected. The sanitizing part fails in case the output doesn't end with a new > line (\n). The sanitizing bit doesn't fail per se, as it is only supposed to remove the first occurring newline char. See below. > Can you cite a command that works for this patch? I don't see ip nor > ipconfig as an example. This may mean that the user needs to craft his own > script, which defeats the purpose of an option. Can't cite a command that works as probably there isn't one but I've attached one I've created myself. The script is simple and all it does is look to see whether the eth0 interface is up and an IP is assigned, in which case it echo(es) 'eth0', otherwise echo(es) 'wlan0'. Now, this is where the sanitizing bit comes into play as 'echo' always appends a newline char unless 'echo -n' is used. In the former case, a way of removing newline chars is needed or the interface name will contain the newline char and the plugin will consequently fail to retrieve information about the intended interface. > > As a user it's impossible to guess what this new option is about. I believe this option is more aimed at power users, especially those on the move who use different network interfaces depending on the circumstances. The user is free to create a script that works for his/her needs and instructs netload to monitor the intended interface without the user having to manually setup the plugin each time a different interface is used. I agree, though, that perhaps a tooltip should be added containing a brief, objective explanation as to what the command does. However, I've made the network interface field (in the configuration box) toggle between enabled/disabled state, depending on whether run-command is enabled, as a sort of hint as to what run-command does. > > Finally it's merely a duplicate of bug 3880, or bug 7431. > > Your way of trying to discover the interface is not good enough, and is therefore rejected.
Bugzilla wouldn't allow me to attach script; pasting it here. ----- #!/bin/bash test -z `ifconfig eth0|grep -i 'inet addr'` && echo 'wlan0' || echo 'eth0'
Hi Miguel, This is not portable (on my system I have wlan0 and em1), and since the user needs to craft his own script it's a no-go. As a power-user you might do something like: sed -i "s/\(Network_Device=\).*/\\1$(test -z `ifconfig em1|grep -i 'inet addr'` && echo 'wlan0' || echo 'em1')/" ~/.config/xfce4/panel/netload-*.rc kill -USR1 $(pidof xfce4-netload-plugin) But as a user you don't want to, hence this option doesn't have its place here. Of course it could be interesting to have the user select the interface in the option dialog, and why not also in the context menu of the panel plugin (right click). It could also try to detect which interface is up and have an option to automatically select that interface. Well, the point of this bug is to reduce the effort to select the right network interface for the user, and as already pointed out there are two bugs for this issue: bug 3880, or bug 7431 (this one is more close to bug 7431). Regards, Mike
(In reply to comment #4) > Hi Miguel, > > This is not portable (on my system I have wlan0 and em1), and since the user > needs to craft his own script it's a no-go. > > As a power-user you might do something like: > > sed -i "s/\(Network_Device=\).*/\\1$(test -z `ifconfig em1|grep -i 'inet > addr'` && echo 'wlan0' || echo 'em1')/" ~/.config/xfce4/panel/netload-*.rc > kill -USR1 $(pidof xfce4-netload-plugin) > I like this way! :) > But as a user you don't want to, hence this option doesn't have its place > here. > > Of course it could be interesting to have the user select the interface in > the option dialog, and why not also in the context menu of the panel plugin > (right click). It could also try to detect which interface is up and have an > option to automatically select that interface. Good idea! > > Well, the point of this bug is to reduce the effort to select the right > network interface for the user, and as already pointed out there are two > bugs for this issue: bug 3880, or bug 7431 (this one is more close to bug > 7431). Makes perfect sense, Mike. Thanks!