New menu of syntax highlighting in the status bar
ClosedPublic

Authored by nibags on May 2 2019, 11:07 AM.

Details

Summary

Now there are more than 300 languages ​​for syntax highlighting and the current menu to select them isn't comfortable, since it's slow to find any language. That is why I propose a new menu with scrollable list plus search bar. This menu is accessible only from the status bar and is equivalent to the "Mode" menu.

After:



Before:

Some features:

  • You can type in the search bar and browse the list with the keyboard, simultaneously.
  • The menu is kept synchronized with the classic "Mode" menu.
  • When pressing Enter or clicking, a syntax highlight is selected and the menu is hidden; when pressing Ctrl + Enter, highlighting is selected keeping the menu visible.
  • The items of languages ​​are separated by sections, this keeps the list more ordered. The search bar has special features:
    • Alphabetical search when writing a single character. For example, when looking for a, all the items that begin with "a" are displayed, this allows you to search for items alphabetically in a more comfortable way.
    • Search by extensions: when writing a file extension, the corresponding language is displayed. For example, when writing cpp, "C ++", "ISO C ++", "SystemC" are showed; or when typing jsx "JavaScript React" is displayed. It also serves by putting a point above, eg.: .h, .y, .te, etc.

Other changes in this diff:

  • Before, in the classic "Mode" menu, the sections and names of languages ​​were arranged alphabetically according to the original names, not the translated names. Now they are sorted according to the translated names.
  • The "Normal" item in the Mode menu is now translated, since nowhere in the code is it translated.
  • Before, in the "Highlighting" menu, the section names and the "None" item weren't translated, now they are translated.
  • When displaying the classic Mode menu, it uses the traduced names, but it verifies the mode selected by the original names in sections. This means, that in languages ​​other than English, the current Mode isn't showed selected (with the radio icon) in the menu. That is corrected.

Tests:

  • The menu was tested on Plasma with KDE Neon with X11 and Wayland.
  • When changing Kate's color theme, the menu adapts correctly to the new colors.

Diff Detail

Repository
R39 KTextEditor
Branch
new-mode-menu
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11581
Build 11599: arc lint + arc unit
nibags created this revision.May 2 2019, 11:07 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptMay 2 2019, 11:07 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
nibags requested review of this revision.May 2 2019, 11:07 AM
nibags edited the summary of this revision. (Show Details)May 2 2019, 11:13 AM
nibags added reviewers: KTextEditor, Kate.
loh.tar added a subscriber: loh.tar.May 2 2019, 3:55 PM

Wow, look very nice :-)
The only thing I would like to have is an auto generated "most often used" list. That was also suggested somewhere. This way you could quickly change between preferred styles instead to search for them.

Your code looks lovely and well documented. However, I always try to establish a docu style to avoid the use of multiline comments /**/ in the code itself and use // style. It doesn't waste so much space.

Will try this later.

src/mode/katemodemenulist.cpp
46

perhaps static instead of inline?

102

coding style: brace always single liner

115

use new style

ngraham added a reviewer: VDG.May 2 2019, 5:28 PM
ngraham added a subscriber: ngraham.May 2 2019, 5:44 PM

OMG so much better! Adding a search field is a godsend.

I have a few UI comments:

  • When the menu is opened, clear any text in the search field left over from a prior search
  • Single-clicking on an item in the list should select it and close the menu; double-click doesn't gain us anything here

When the menu is opened, clear any text in the search field left over from a prior search

For my taste would that the usability only worsen, this way may not be usual but works charming. Perhaps may a change to select the field in that case a possibility, so you could clear it easily

Single-clicking on an item in the list should select it and close the menu; double-click doesn't gain us anything here

hm, works here exact this way (?)

Some comments/ideas, no real criticism

  • .cp does not show c++, same for cp
  • These ctrl-return is handy! Would be good to have this on right or middle too or in conjunction with some modifier key alt/ctrl/shift
  • The blue frame (in dark theme) looks compared to the other plain menus slightly strange, any possibility to remove this?
  • Is your new great menu easy usable for other parts? Like the encoding?
  • As said: Would like to see these history stuff as part of this menu

Great work!

ngraham added a comment.EditedMay 2 2019, 10:07 PM
  • The blue frame (in dark theme) looks compared to the other plain menus slightly strange, any possibility to remove this?

This is a generic issue with the selection border being very bright in the Breeze Dark theme and should be solved there, not worked around here.

As first feedback: much better than current state :)

nibags added a comment.EditedMay 3 2019, 11:46 AM

Thanks for the comments, in a while I will update the diff with the suggestions/corrections.

The only thing I would like to have is an auto generated "most often used" list. That was also suggested somewhere. This way you could quickly change between preferred styles instead to search for them.

I could add a "Frequents"/"Most often used" section to the beginning of the menu, it would be useful.

When the menu is opened, clear any text in the search field left over from a prior search

Also I could keep all the search text as selected, to delete it with a single button.
I hope more comments about it, I'm not sure which will be the best option: S

Single-clicking on an item in the list should select it and close the menu; double-click doesn't gain us anything here

To me the double-click works correctly. It may also be necessary to add a special case for double-click to avoid problems.

.cp does not show c++, same for cp

I checked the XML file of "C++", and *.cp isn't an extension associated with that language, so it isn't displayed.

These ctrl-return is handy! Would be good to have this on right or middle too or in conjunction with some modifier key alt/ctrl/shift

Good idea! I will also add shift and alt and combinations.

The blue frame (in dark theme) looks compared to the other plain menus slightly strange, any possibility to remove this?

You can erase the edge, but I'm not sure if the brightness. I will try, although I don't know if it will be a good idea, since that brightness indicates that the widget is in focus.
As @ngraham says, it might be better to reduce the brightness in the Breeze Dark theme.

Is your new great menu easy usable for other parts? Like the encoding?

It could be done. I would have to separate the menu into two classes that are inherited, one with the menu only, so that it can be implemented elsewhere.

As first feedback: much better than current state :)

;-D

Single-clicking on an item in the list should select it and close the menu; double-click doesn't gain us anything here

To me the double-click works correctly. It may also be necessary to add a special case for double-click to avoid problems.

Double-click is not the correct UX here. Here is the rule for when something should be double-clickable:

  • Is the item selectable, and when selected, you can perform more than one action with it?
    • If so, make it double-clickable and expose those actions while it is selected.
    • If not, because the only action available is "activate this item", make it only single-clickable, even when using the systemwide double-click setting. This is for example why buttons are always single-clickable: if they were selectable, there would still be only one thing you can do with them ("activate").

I need to finally put this in a HIG page...

I could add a "Recent" section to the beginning of the menu, it would be useful.

Nice! Just to be clear. That should to be work over session borders, not only the current one.

The blue frame (in dark theme) looks compared to the other plain menus slightly strange, any possibility to remove this?

You can erase the edge, but I'm not sure if the brightness..., since that brightness indicates that the widget is in focus.

These focus is in general fine, and I didn't wanted to poke around here to get rid of it. Just to use perhaps some differend construction of these popup. Know idea what exactly :-) These "edge" may enough.

Is your new great menu easy usable for other parts? Like the encoding?

It could be done.

Nice! Maybe on Kde-Wide level? :-) But Kate-Only as first step may great enough.

.cp does not show c++, same for cp

I checked the XML file of "C++", and *.cp isn't an extension associated with that language, so it isn't displayed.

It's a little strange that while enter "c-p-p" it is shown/gone/shown iirc. How about some fuzzy search?

anthonyfieroni added inline comments.
src/mode/katemodemenulist.cpp
66

As well for icon size.

130

You can make a static const int iconSize = 16 or something to use it everywhere?

142

Here as well, if it's 16/2

267

Ditto

408

Ditto

dymanic_cast is expensive, prefer qobject_cast if you want to check result against nullptr, if you don't want - static_cast.

My comments are very vague, but I think this patch could even be better. Only did a review half through due to lack of time.

src/mode/katemodemenulist.h
142

Can you make this a QListView? I once heard the QListWidget will be deprecated in Qt6, rule of thumb is: always use Q*View instead of Q*Widget, since this then will also work in QML.

176

This should use QStandardItem, iiuc.

nibags updated this revision to Diff 57561.May 4 2019, 5:07 PM
    • Update:
  • Select the search text when showing the menu.
  • Also use Shift, Alt or Meta + Return to select an item without hiding the menu.
  • Fix style in signals/slots.
  • Use constant variable for icon size.
  • Use static_cast instead of dynamic_cast.
  • Always use brackets in if/else conditions.
  • Decrease unnecessary multiline comments.
  • Update base.
nibags added a comment.May 4 2019, 5:16 PM

Can you make this a QListView? I once heard the QListWidget will be deprecated in Qt6, rule of thumb is: always use Q*View instead of Q*Widget, since this then will also work in QML.

I used QListWidget because it's more simpler to implement, but in that case it would be better to use QListView. I'm going to check it to do it!

nibags marked 8 inline comments as done.May 4 2019, 5:18 PM
ngraham requested changes to this revision.May 5 2019, 7:08 PM

Still needs the menu items to apply and close on single-click even when using the systemwide double-click mode. The option itself in System Settings Provides a clue regarding when it's appropriate to use double-click:

Double-click to open files and folders (single-click to select)

This isn't a file or a folder. Single-clicking on a menu item should always choose that thing and close the menu.

This revision now requires changes to proceed.May 5 2019, 7:08 PM
nibags updated this revision to Diff 57618.May 5 2019, 9:10 PM
  • Allow to select an item with a single click, regardless of the configuration of the system.

I had forgotten that it's possible to change the settings for the click/double-click actions, now I correct it.
Then it looks like this:

  • Return/Enter or One-click: select an item and hide the menu.
  • Double-click: no action (that is, it acts as a single click).
  • Ctrl, Alt, Shift or Meta + Return: select an item without hiding the menu.
ngraham accepted this revision.May 6 2019, 7:34 PM

Thanks, this is now perfect from my UI perspective. I'll let the KTextEditor folks do the code review part. :)

This revision is now accepted and ready to land.May 6 2019, 7:34 PM
cullmann accepted this revision.May 7 2019, 7:37 AM

I am ok with having this as is.
If one can move it to QListView, nice, but that could be done in an extra request.
Thanks for this great improvement.

I'm going to move it to QListView (or QTreeView) in a future diff, I have checked and it isn't complicated to do.

Now I want to test it a bit more before doing the commit.

Ok, thanks!

I could add a "Recent" section to the beginning of the menu, it would be useful.

Nice! Just to be clear. That should to be work over session borders, not only the current one.

Should you do it, here some bug report https://bugs.kde.org/show_bug.cgi?id=397563 but perhaps can this be closed in any case

nibags updated this revision to Diff 57819.May 9 2019, 8:06 PM
  • Fixes:

Some fixes for locations:

  • Before, the word wrap was only applied in spaces and, in languages such as German, there are large words that pass under the scroll bar. This is corrected.
  • Improves the alignment of the menu with respect to the trigger button in languages with Right-to-left layouts, such as Arabic or Hebrew. In those cases the menu is aligned to the left of the button (the idea is that the menu is aligned towards the center of the window, since it's shown on the edge of the window).

Fixes in the search:

  • Don't scroll to the selected item when calling the function clear(), if the search is already empty.
nibags updated this revision to Diff 57841.May 10 2019, 8:36 AM
  • Fixes the menu alignment of the previous update
nibags updated this revision to Diff 57905.May 11 2019, 1:21 PM
  • Add delimiters « »

Now it's OK

nibags closed this revision.May 11 2019, 8:15 PM

Btw, I think this is a really nice improvement! Keep it coming :-)

nibags marked 2 inline comments as done.Jun 10 2019, 8:08 AM

I changed QListWidget by QListView in D21712

Coding style wise, could one change:

  1. this->XXX() => XXX()
  2. void functions should not have "return;" at the end, we don't do that