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

Feature request for a "shelf" option ( #51) #52

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
dragon

# clangd
compile_commands.json
.cache/
Comment on lines +2 to +5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO these should go into user's global ignore file rather than on the project's one.

3 changes: 3 additions & 0 deletions dragon.1
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ to drag out or drop files into, instead.
.B -x --and-exit
exit after a single completed drop.
.TP
.B -X --individual-exit
exit after all items are dropped off. (Use with multiple files)
.TP
.B -t --target
act as a target instead of source.
.TP
Expand Down
59 changes: 55 additions & 4 deletions dragon.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ char *progname;
bool verbose = false;
int mode = 0;
int thumb_size = 96;
bool and_exit;
bool keep;
bool print_path = false;
bool icons_only = false;
bool always_on_top = false;

static enum { QUIT_NONE, QUIT_ALL, QUIT_ITEM } quit_mode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the cli arg will be kept --individual-exit then probably should rename QUIT_ITEM to QUIT_INDIVIDUAL.


static char *stdin_files;

#define MODE_HELP 1
Expand All @@ -67,6 +68,7 @@ GtkWidget *all_button;
// ---

void add_target_button();
GtkWidget* find_child(GtkWidget* parent, const gchar* name);

void do_quit(GtkWidget *widget, gpointer data) {
exit(0);
Expand Down Expand Up @@ -115,6 +117,8 @@ void drag_data_get(GtkWidget *widget,
}

void drag_end(GtkWidget *widget, GdkDragContext *context, gpointer user_data) {
uri_count--;
Copy link
Contributor

@FichteFoll FichteFoll Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break target mode together with shelf mode (or 'drag once mode') and cause URIs to be overridden as that uses the uri_count counter for indexing. Besides, imo it would be better to have this near the code that is responsible for handling individual entiries when quit_mode == QUIT_ITEM.

And finally, what if -X is used together with --all?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you mean i should decrement uri_count near the if statement ? Or should i make it use a different counter

struct draggable_thing *dd = (struct draggable_thing *)user_data;
if (verbose) {
gboolean succeeded = gdk_drag_drop_succeeded(context);
GdkDragAction action = gdk_drag_context_get_selected_action (context);
Expand All @@ -137,8 +141,51 @@ void drag_end(GtkWidget *widget, GdkDragContext *context, gpointer user_data) {
if (action_str[0] == 'i')
free(action_str);
}
if (and_exit)

if (quit_mode == QUIT_ALL){
gtk_main_quit();
}

if (quit_mode == QUIT_ITEM) {
Comment on lines +145 to +149
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after ). Also since they are mutually exclusive, else if makes more sense IMO.

Suggested change
if (quit_mode == QUIT_ALL){
gtk_main_quit();
}
if (quit_mode == QUIT_ITEM) {
if (quit_mode == QUIT_ALL) {
gtk_main_quit();
} else if (quit_mode == QUIT_ITEM) {

if (uri_count == 0) {
gtk_main_quit();
return;
}

GtkWidget* button = find_child(vbox,dd->text);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after the comma.

Suggested change
GtkWidget* button = find_child(vbox,dd->text);
GtkWidget* button = find_child(vbox, dd->text);

if (button != NULL) {
gtk_container_remove(GTK_CONTAINER(vbox), button);
} else {
fprintf(stderr, "Could not find button with label: %s",dd->text);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fprintf(stderr, "Could not find button with label: %s",dd->text);
fprintf(stderr, "Could not find button with label: %s", dd->text);

}
}
}

// https://stackoverflow.com/a/23497087
GtkWidget* find_child(GtkWidget* parent, const gchar* name) {
if (GTK_IS_BUTTON(parent)) {
if (g_ascii_strcasecmp(gtk_button_get_label(GTK_BUTTON(parent)), (gchar*)name) == 0) {
return parent;
}
}

if (GTK_IS_BIN(parent)) {
GtkWidget *child = gtk_bin_get_child(GTK_BIN(parent));
return find_child(child, name);
}

if (GTK_IS_CONTAINER(parent)) {
GList *children = gtk_container_get_children(GTK_CONTAINER(parent));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for this to return NULL ? If it is then it's going to lead to a null-deref down below.

do {
GtkWidget* widget = find_child(children->data, name);
if (widget != NULL) {
return widget;
}
} while ((children = g_list_next(children)) != NULL);

}

return NULL;
}

void add_uri(char *uri) {
Expand All @@ -151,6 +198,7 @@ void add_uri(char *uri) {
}

GtkButton *add_button(char *label, struct draggable_thing *dragdata, int type) {

GtkWidget *button;

if (icons_only) {
Expand Down Expand Up @@ -359,7 +407,7 @@ drag_data_received (GtkWidget *widget,
} else if (verbose)
fputs("Received nothing\n", stderr);
gtk_drag_finish (context, TRUE, FALSE, time);
if (and_exit)
if (quit_mode == QUIT_ALL)
gtk_main_quit();
}

Expand Down Expand Up @@ -485,7 +533,10 @@ int main (int argc, char **argv) {
mode = MODE_TARGET;
} else if (strcmp(argv[i], "-x") == 0
|| strcmp(argv[i], "--and-exit") == 0) {
and_exit = true;
quit_mode = QUIT_ALL;
} else if (strcmp(argv[i], "-X") == 0
|| strcmp(argv[i], "--individual-exit") == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of a long name for a cli arg, something simpler might be better. But I guess not that big of a deal since there's the short form -X.

Copy link
Contributor

@FichteFoll FichteFoll Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming isn't exactly intuitive to me. I wouldn't expect this option to cause the application to exit once all the individual items have been dragged out of it.

How about naming it --item-remove, --remove-individual, --once-each or similar and then use this option together with --and-exit to trigger an exit once all items have been moved out? Whereas if the option wasn't specified, --and-exit would still cause a quit after the first successful drag.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah --once-each sounds like a better naming option to me as well.

quit_mode = QUIT_ITEM;
} else if (strcmp(argv[i], "-k") == 0
|| strcmp(argv[i], "--keep") == 0) {
keep = true;
Expand Down