use position of edit context menu for preview actions#2803
use position of edit context menu for preview actions#2803octaeder wants to merge 3 commits intotexstudio-org:masterfrom octaeder:cursorPreview
Conversation
src/latexeditorview.cpp
Outdated
| if (act->objectName().endsWith("removePreviewLatex")) { | ||
| act->setData(posInDocCoordinates); | ||
| isPicMenu = true; | ||
| act->setData( QVariantList() << QVariant(posInDocCoordinates) << QVariant(isPicMenu) ); |
There was a problem hiding this comment.
there should not be a necessity to this list.
Especially as the first element type basically depends on the second makes the code hard to read.
Simplest solution is, to use already line/col at this point instead of coordinate which makes a later distinction unnecessary.
src/latexeditorview.cpp
Outdated
| { | ||
| // copy managed action "previewLatex" from texstudio.cpp | ||
| // QAction *act = new QAction(LatexEditorView::tr("Pre&view Selection/Parentheses")); | ||
| QAction *act = new QAction(); |
There was a problem hiding this comment.
I am really skeptical about this copying actions stuff.
Basically you have 2 actions with the same short cut, not sure that this leads to well defined behavior when one of them is discarded.
And again, code duplication ... 3 nearly identical for loops...
There was a problem hiding this comment.
probably the cleanest solution for this is, that the context menu point is stored in DefaultInput... as extra variable and declared invalid/deleted when the context menu is closed.
texstudio checks in edView if the point is present and act accordingly.
This would avoid any hacks on the actions.
There was a problem hiding this comment.
I can't use shortcuts while the (contex) menu is open. Especially pressing the Alt-key immediately closes the (context) menu. So I conclude that the (context) menu shows the shortcut merely as information. If you remove the shortcut from the main menu action (config dialog) it will no longer be present in the context menu.
Do we know that the context menu is present as long as the triggered action is running?
Thesaurus, as an example, is in the main menu with shortcut but there is a new action created in latexeditorview with no shortcut. The new action is used to pass coordinate information to the final function. But the preview actions are in baseActions. These are added as is (in the main menu) to the context menu. So they show the same (probably translated) text and the same shortcut. "Copying" them into new actions repeats the procedure used for Thesaurus and avoids repeating translations and shortcuts, but allows for new slots as with Thesaurus.
I didn't manage to write a function for the loops, sorry.
Thus I would prefer to stay with this solution. Could you provide a case that you assume to have issues with the solution?
Thx
|
I have taken your PR and adapted it not to use the QAction manipulation which I find suspicious/hard to explain what actually happens. (QActions are global constructs, with global shortcuts. So copying results in 2 actions with the same shortcut. Are Actions automatically deleted after use or do we actually have a memory leak? This things may work on one or all systems (win/osx/linux/Qt5/Qt6) but not necessarily, so better avoid these questions. The resulting code is functionally identical. |
|
to answer some of the questions above. |
This PR resolves #2794. This is done by replacing in the context menu the original main menu entries with entries which use new slots. These slots pass the coordinates of the context menu to the preview/remove preview functions. The latter also needs information what type the coordinates are, because different coordinate systems are used for the editor and the inline preview.
BTW it seems that no one ever noticed that two entries of the preview context menu are not translatable. This should be possible now either:
Key accelerators for these are also added.