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

add minimap option #536

Merged
merged 1 commit into from Jul 31, 2020
Merged

add minimap option #536

merged 1 commit into from Jul 31, 2020

Conversation

mbkma
Copy link
Member

@mbkma mbkma commented Apr 13, 2020

Edit: I think it is done now, I fixed remaining issues, taking another approach for implementing the minimap.
Fixes #440
Screenshot at 2020-05-02 00-51-51

@raveit65
Copy link
Member

I think the warning should be fixed.

@raveit65 raveit65 requested a review from sc0w April 21, 2020 14:23
@mbkma
Copy link
Member Author

mbkma commented Apr 21, 2020

yes, you do not have to review this right now, because I am still working on it.

@mbkma mbkma marked this pull request as ready for review May 1, 2020 22:55
@sc0w
Copy link
Member

sc0w commented May 2, 2020

@mbkma

17 commits behind master, please, can you rebase it?

@mbkma
Copy link
Member Author

mbkma commented May 2, 2020

@sc0w done

pluma/pluma-view.c Outdated Show resolved Hide resolved
pluma/pluma-view.c Outdated Show resolved Hide resolved
@mbkma
Copy link
Member Author

mbkma commented May 21, 2020

I basically did a git checkout origin/master pluma_view.c, a git rebase upstream/master and added

	tab->priv->view_map_frame = gtk_frame_new (NULL);
	map = gtk_source_map_new();

+	GtkCssProvider *provider = gtk_css_provider_new ();
+	gtk_css_provider_load_from_data (provider,
+					 "textview { font-family: Monospace; font-size: 1pt; }",
+					 -1,
+					 NULL);
+	gtk_style_context_add_provider (gtk_widget_get_style_context (map),
+					GTK_STYLE_PROVIDER (provider),
+					GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
+	g_object_unref (provider);

	gtk_source_map_set_view (GTK_SOURCE_MAP(map), GTK_SOURCE_VIEW(tab->priv->view));
	gtk_container_add (GTK_CONTAINER(tab->priv->view_map_frame), map);
	gtk_widget_show (tab->priv->view_map_frame);

to PlumaTab.

@rbuj
Copy link
Contributor

rbuj commented May 21, 2020

It works fine, although display-overview-map key is not managed in pluma-prefs-manager[.h|.c]

0001-add-overview-map-option.patch.gz

@rbuj
Copy link
Contributor

rbuj commented May 21, 2020

From my point of view, I think that the Overview Map should be enabled/disabled from the View menu not from the preferences dialog.

@raveit65
Copy link
Member

yes, you do not have to review this right now, because I am still working on it.

Any news or progress?

@mbkma
Copy link
Member Author

mbkma commented Jun 13, 2020

Sadly I do not have a lot of time at the moment. It seems to me, that it is very complicated to add the show/hide option to the View menu via the pluma-prefs-manager, as opposed to the settings dialog, where I can simply use g_settings_bind.

Moving the show/hide option into the Menu the difficulty for me is the following: The overview map is created in the PlumaTab class for each tab individually. MenuItem callbacks are located inside pluma-commands-*.c file. From within pluma-commands-view.c I can only access PlumaWindow, although I would need to do sth. like gtk_widget_show(overviewmap) for all open tabs in the current window.

That said I would prefer the setting in the View menu.

Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

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

This does work, though the conflicts with master need to be addressed before it can be merged. Pluma with this applied can optionally display a small-resolution overview of the entire file

@mbkma
Copy link
Member Author

mbkma commented Jun 22, 2020

Should it be merged? I think it is fully functional, despite it does not use pluma-prefs-manager. Is it possible to handle all prefs with g_settings_bind? Gedit removed gedit-prefs-manager a while ago.

I don't know when I have time to move the show/hide option from pluma-preferences-dialog into the View menu. Of course this could be done in the future as well.

@sc0w
Copy link
Member

sc0w commented Jun 22, 2020

github says: This branch has conflicts that must be resolved

@mbkma
Copy link
Member Author

mbkma commented Jun 22, 2020

The question still remains if you consider this PR or if I should close it and try to implement the minimap with the show/hide option in the View menu in the future.

@sc0w
Copy link
Member

sc0w commented Jun 22, 2020

@mbkma

The question still remains if you consider this PR or if I should close it and try to implement the minimap with the show/hide option in the View menu in the future.

more improvements can be done in future PRs, it isn't reason to block this PR.

I will review the code/behavior again with your latest changes since my latest review.

@sc0w
Copy link
Member

sc0w commented Jun 22, 2020

@mbkma

See ead4c34

You can use pluma_prefs_manager->settings instead new gsettings variable ?

@mbkma
Copy link
Member Author

mbkma commented Jun 23, 2020

@sc0w done

@raveit65
Copy link
Member

Pluma-overview

For me the overview is too small. It should be possible to resize them (left from the scrollbar), so this isn't really usable for me with or without my glasses. But this isn't a blocker by me.

A menu entry would be great, because it's a similar UI element like sidebar and others, but this can be done in another PR if someone wishes this.

Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

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

Builds and runs fine.

@sc0w
Copy link
Member

sc0w commented Jun 29, 2020

the behavior with drag & drop inside the minimap is a little weird working with long files (50k lines)

for example copy paste this log into pluma:

https://api.travis-ci.org/v3/job/701187611/log.txt

I only notice this warning in the logs...

pluma-prefs-manager.h:85: Warning: Pluma: symbol='GPM_DISPLAY_OVERVIEW_MAP': Unknown namespace for symbol 'GPM_DISPLAY_OVERVIEW_MAP'

...but I think it can be fixed with more similar warnings in other PR.

@raveit65
Copy link
Member

@mbkma
Here are open questions, before we start with new PRs.

@mbkma
Copy link
Member Author

mbkma commented Jul 12, 2020

@raveit65 The new PR will make this PR easier. I can rebase this PR after new PR is merged.

@raveit65
Copy link
Member

@mbkma
Ok, so in which order you like your PRs to be reviewed?

@raveit65
Copy link
Member

@mbkma
Please rebase your PR.

@mbkma
Copy link
Member Author

mbkma commented Jul 31, 2020

the behavior with drag & drop inside the minimap is a little weird working with long files (50k lines)

@sc0w Yes, but I suppose that this is a problem from GtkSourceView, because it happens in older versions of Gedit (which still have the minimap) as well.
Btw, #563 fixed pluma-prefs-manager.h:85: Warning: Pluma: symbol='GPM_DISPLAY_OVERVIEW_MAP': Unknown namespace for symbol 'GPM_DISPLAY_OVERVIEW_MAP' and related warnings.

@raveit65
Copy link
Member

Works for me like before. Ready to go?

@sc0w sc0w requested review from sc0w and removed request for sunweaver July 31, 2020 14:03
Copy link
Member

@sc0w sc0w left a comment

Choose a reason for hiding this comment

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

no new warnings in the logs

@sc0w sc0w merged commit 3c2bf1a into mate-desktop:master Jul 31, 2020
@mbkma mbkma deleted the minimap branch July 31, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add a minimap option.
5 participants