Skip to content

commander: small improvement on listing and panel width #1394

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

commander: small improvement on listing and panel width #1394

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 29 additions & 5 deletions commander/src/commander-plugin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,16 @@ store_populate_menu_items (GtkListStore *store,
} else {
gchar *tmp;
gchar *tooltip;
gchar *label = g_markup_printf_escaped ("<big>%s</big>", item_label);
gchar *basename = g_path_get_basename (item_label);
gchar *dirname = g_path_get_dirname (item_label);
gchar *dirname_basename = g_path_get_basename(dirname);
gchar *label;

if (g_strcmp0(".", dirname_basename) == 0) {
label = g_markup_printf_escaped ("<big>%s</big>", basename);
} else {
label = g_markup_printf_escaped ("<big>%s/%s</big>", dirname_basename, basename);
}
Comment on lines -348 to +357
Copy link
Member

Choose a reason for hiding this comment

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

This seems overly fragile to me, as menu item labels can contain pretty much anything, including /s.

Take for example Edit → Insert Date → yyyy/mm/dd: with this change, it appears as mm/dd.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that may be a problem, and in that case, it's also possible to see all details in the smaller part of the menu item ( Edit → Insert Date → yyyy/mm/dd).


tooltip = gtk_widget_get_tooltip_markup (node->data);
if (tooltip) {
Expand All @@ -365,6 +374,9 @@ store_populate_menu_items (GtkListStore *store,
-1);

g_free (label);
g_free (basename);
g_free (dirname);
g_free (dirname_basename);
}

g_free (item_label);
Expand Down Expand Up @@ -410,8 +422,11 @@ fill_store (GtkListStore *store)
/* open files */
foreach_document (i) {
gchar *basename = g_path_get_basename (DOC_FILENAME (documents[i]));
gchar *label = g_markup_printf_escaped ("<big>%s</big>\n"
gchar *dirname = g_path_get_dirname (DOC_FILENAME (documents[i]));
gchar *dirname_basename = g_path_get_basename (dirname);
gchar *label = g_markup_printf_escaped ("<big>%s/%s</big>\n"
"<small><i>%s</i></small>",
dirname_basename,
Comment on lines +425 to +429
Copy link
Member

Choose a reason for hiding this comment

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

Here it's not as problematic as those are paths. However, the dirname is not necessarily helping much as it might very well still be the same (e.g. if there are foo/src/main.c and bar/src/main.c, this change won't help).

If this is really an issue you encounter, I suggest investigating a more clever solution, possibly verifying there are effectively duplicates, and then compute the difference between the paths in order to show parts that are actually relevant. IIRC there's something like that in Geany's got to popup you might get inspiration from.

Once that's dealt with I wonder what the best visual layout would be… maybe <big>$basename</big> <small>($path_diff)</small>\n<small><i>$path</i></small>? Not tested though, but I think preserving focus on the filename, or actually what's really relevant in it, is important. So maybe even highlight the different parts?

Copy link
Author

@icy icy Feb 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that your suggestion would be better and would address all cases well.

I was actually thinking if I could do some computing for all items before getting the best display option. However, I think that would require more resources, and I don't think it's not worth trading. I was really getting some hard time using our plugin, and the best idea I've been able to share, is this (wip) pull request.

I will spend a little more time on this, at least for my own usage. If you think there should be more efficient way to address the problem, I can help to open an issue and hopefully someone from the community can help.

Thank you a lot for your time.

basename,
DOC_FILENAME (documents[i]));

Expand All @@ -422,6 +437,8 @@ fill_store (GtkListStore *store)
COL_DOCUMENT, documents[i],
-1);
g_free (basename);
g_free (dirname);
g_free (dirname_basename);
g_free (label);
}
}
Expand Down Expand Up @@ -658,10 +675,17 @@ create_panel (void)
GtkWidget *scroll;
GtkTreeViewColumn *col;
GtkCellRenderer *cell;

int window_width;
int window_height;

gtk_window_get_size (GTK_WINDOW(geany_data->main_widgets->window), &window_width, &window_height);
if (window_width >= 500) {
window_width = window_width / 2;
}
Comment on lines +682 to +684
Copy link
Member

Choose a reason for hiding this comment

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

Is 500 really too big? Otherwise if the goal is to make it bigger, then the threshold should be whatever makes it larger than 500 (math suggests it'd be 1000).

Also note that this is not dynamic during a Geany session, it's only the default size when opening the panel for the first time, if you change the Geany window size later it won't affect this. This is not necessarily an issue though.

Copy link
Author

Choose a reason for hiding this comment

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

Is 500 really too big? Otherwise if the goal is to make it bigger, then the threshold should be whatever makes it larger than 500 (math suggests it'd be 1000).

When the main window has some moderate size , I want(ed) the commander's panel has relatively smaller size. When the main window is too small (< 500), I want to see the commander of the same size (otherwise it's too small to show any thing if I get it right.)

I admit my calculation is bad. It's better that I used some min() function instead.

Also note that this is not dynamic during a Geany session, it's only the default size when opening the panel for the first time, if you change the Geany window size later it won't affect this. This is not necessarily an issue though

I've also noticed that, thanks for your confirmation.


plugin_data.panel = g_object_new (GTK_TYPE_WINDOW,
"decorated", FALSE,
"default-width", 500,
"default-width", window_width,
"default-height", 200,
"transient-for", geany_data->main_widgets->window,
"window-position", GTK_WIN_POS_CENTER_ON_PARENT,
Expand Down Expand Up @@ -804,4 +828,4 @@ void
plugin_help (void)
{
utils_open_browser (DOCDIR "/" PLUGIN "/README");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Please bring the trailing newline back 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Sure I will do that. This would be a bug somewhere in (my) Geany system.

OT: I was into this issue #1394 (comment), now I have two options at project level (Ensure new line at file end [checked] + Strip trailing spaces and tabs [unchecked]). Somehow only one of them works as expected :(