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 contextual menu for file encoding in status bar #5523

Closed

Conversation

@vlakoff
Copy link
Contributor

commented Apr 13, 2019

Fixes #400 and #756.
That would be a simple but very convenient feature.

The implementation seems to be very easy.
The only thing I can't say is if it works correctly on the "Character sets" submenu. If yes, that's wonderful!

@chcg chcg added the enhancement label Apr 14, 2019

@@ -418,23 +418,23 @@ BOOL Notepad_plus::notify(SCNotification *notification)
if (notification->nmhdr.hwndFrom == _statusBar.getHSelf())
{
LPNMMOUSE lpnm = (LPNMMOUSE)notification;
if (lpnm->dwItemSpec == DWORD(STATUSBAR_CUR_POS))
if (lpnm->dwItemSpec == DWORD(STATUSBAR_DOC_TYPE))

This comment has been minimized.

Copy link
@chcg

chcg Apr 14, 2019

Contributor

Is there a reason why STATUSBAR_DOC_TYPE and STATUSBAR_CUR_POS are swapped?

This comment has been minimized.

Copy link
@vlakoff

vlakoff Apr 14, 2019

Author Contributor

There are 2 commits in this PR. The first commit is code reordering for readability, the second commit is the actual change.

This comment has been minimized.

Copy link
@chcg

chcg Apr 14, 2019

Contributor

Thanks for the hint: "Sort conditionals like the display order" explains it all :-)

@vlakoff

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

Would someone be very kind, to compile with this change and try it?

@chcg

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

@vlakoff A compiled version is already available with the appveyor build, see e.g. for x64: https://ci.appveyor.com/project/donho/notepad-plus-plus/builds/23821550/job/8x46cl6qnj8jm64f/artifacts

@vlakoff

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

Woooo, that's really great to know!

Thus, I tried the binary, and my change seems to be perfectly working :)

@donho

This comment has been minimized.

Copy link
Member

commented Aug 3, 2019

The Encoding menu is too large and complex to show as context menu.
So this feature won't be in Notepad++.

@donho donho closed this Aug 3, 2019

@sasumner

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

Add contextual menu for file encoding in status bar

The title is confusing, but seems to want it in the status bar?

The Encoding menu is too large and complex to show as context menu.

Even Don was confused, and he doesn't mention status bar, although there really isn't room in the status bar for it anyway.

I didn't try it, but can't one edit contextMenu.xml and put what is desired in the right-click-of-editor-window-context-menu, perhaps even in an Encodings > grouping?? That would seem to satisfy as much as having something for it in the status bar, and wouldn't require ANY PR with code changes...

@vlakoff

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2019

Indeed, it's the context menu on the status bar, not on the main window.
See the linked issues, there are demands for this.

Did you at least try it?
The menu works perfectly. And yes, it fits well.

As more, people are usually interested by the "root" encodings (ANSI, UTF-8), they more rarely dig into the submenus (which, again, are perfectly working too).

@xylographe

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

I didn't try it,

By all means, give it a try. This patch makes the encoding in the status bar behave consistent with the end of line mode, and the language : a right click or a double click opens the corresponding menu. The encoding menu is smaller than the language menu, hence lack of room is not an issue.

@sasumner

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

@vlakoff / @xylographe As you can tell, I didn't fully familiarize myself with all the background (which is a bit disjointed, due to age, perhaps) before my recent comment. After doing so, it all seems fine to me. I officially retract all of my earlier comments. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.