gmorer/fuzzel:feat/include_path into master This option include executable from the PATH environment variable to be included in the list.
Will fix #11 (already close tho).
The name sounds like "the path of include files".
I'm not the maintainer, but a clearer name would be more explicit. Maybe "--include-bins-from-path".
Personally, I don't relate to this feature, since all the binaries from $PATH will include mostly results that don't make sense to launch interactively. For the few that do, .desktop files can be added that include icons for them, include other keywords that might be useful to search for, and specify whether they should be launched as GUI apps or in a terminal.
I understand that dmenu_run allowed selecting binaries likethis with dmenu, but I didn't relate to that either.
Hello, yes the name isn't really nice, i was thinking of something with bins/binaries first in it, which would not be 100% acccurate since you can have scripts which are not binaries in your $PATH.
But i can definitely change it to that.
An example of usage for me, is to run scripts that are in my $PATH from the launcher, doesn't look super nice without icons, but since Fuzzel already execute commands supplied even without desktop files, a visual help to see that an executable exist is I think nice.
I was thinking of switching to Fuzzel coming from Rofi that i use with the following command: rofi -show combi which combine desktop and executable.
I think i found a nice on (been thinking about it a lot lately) --list-path-execs, does it sound ok to you @markstos ?
I think the name is better but @dnkl will make the final decison.
I need to think about whether this is a feature I want in fuzzel, but I agree --list-path-execs is a better name than the original one. However, if we're going with such a long name, we might as well make it even more descriptive - perhaps --list-executables-in-path?
Thank you so much for the patch, it was exactly what i was missing, almost went back to dmenu
@ -782,6 +782,9 @@ parse_section_main(struct context *ctx)
else if (strcmp(key, "icons-enabled") == 0)
return value_to_bool(ctx, &conf->icons_enabled);
else if (strcmp(key, "include-path") == 0)
Please rename include-path to either list-executables-in-path, or include-executables-from-path, or something else that is less ambiguous.
list-executables-in-path ❗
@ -0,0 +1,88 @@
#include "path.h"
Indentation is wrong in this entire file; we use 4 spaces in fuzzel.
@ -0,0 +13,4 @@
#include "xdg.h"
#include "char32.h"
void path_find_programs(struct application_list *applications)
nit:
void
path_find_programs(struct application_list *applications)
{
...
}
@ -0,0 +57,4 @@
struct stat st;
if (fstatat(it->item.fd, e->d_name, &st, 0) == -1) {
LOG_WARN("%s: failed to stat", e->d_name);
continue;
wtitle is leaked here.
@ -0,0 +59,4 @@
LOG_WARN("%s: failed to stat", e->d_name);
continue;
}
if (S_ISREG(st.st_mode) && st.st_mode & S_IXUSR) {
wtitle is leaked here, when the if statement is not taken.
I moved the variable creation after checking the file type.
@ -0,0 +63,4 @@
char *exec = malloc(path_size + 1 + strlen(e->d_name) + 1);
strcpy(exec, it->item.path);
exec[path_size] = '/';
strcpy(exec + path_size + 1, e->d_name);
I'd suggest using either snprintf() to build the entire string in a single call, or strcpy() followed by two strcat(). It makes it easier to read, and the small performance boost you get from doing it with strcpy() with offsets just isn't worth it, imho.
@ -0,0 +82,4 @@
tll_foreach(entries, it) {
applications->v[applications->count++] = it->item;
tll_remove(entries, it);
This leaks both the fd you opened in the first for loop, as well as the path element.
even with the xdg_data_dirs_destroy(paths);?
I made changes to only loop once, so the directory fd should now be closed by closedir
@ -0,0 +84,4 @@
applications->v[applications->count++] = it->item;
tll_remove(entries, it);
}
tll_free(entries);
tll_free() isn't necessary when you've tll_remove():ed all elements in the list.
@ -0,0 +2,4 @@
#include "application.h"
#include "icon.h"
#include "tllist.h"
Neither icon.h nor tllist.h are needed, are they?
a72381676a
to 19c168a1f1
19c168a1f1
to 649c6a769c
I made the requested changes (except fd/path leak (not sure it leak)) sorry for the resolved/unresolved spam, not sure I should be the one doing that.
I added an entry in the CHANGELOG.md as well.
649c6a769c
to 06560cdc12
I added a check to not list multiple times an executable with the same name.
Overall, this looks good. A couple of minor issues, see below.
I'm also missing updated shell completions (completions/zsh + completions/fish).
@ -152,3 +152,3 @@
'render.c','render.h',
'shm.c','shm.h',
'stride.h',
'stride.h','path.c',
nit: this list is a) sorted, and b) also includes the corresponding header file
@ -0,0 +60,4 @@
bool already_exist = false;
tll_foreach(entries, it) {
if (c32cmp(wtitle, it->item.title) == 0) {
already_exist = true;
minor performance tweak: break out of the loop after setting already_exist = true.
06560cdc12
to cb95154e50
cb95154e50
to c1fdcb7d2f
Done
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?