Reimplement properly bidi icons

When the cursor in RTL text, icons for "depth-increment" or
"layout-toggle Enumerate" look wrong.

Instead of relying on the clumsy "bidi" lfun of 2898c335, this new
version relies on a new Toobar tag BidiItem that inserts an action
which can have two icons, depending on the direction of the paragraph
containing the caret (or of the direction of the UI when no file is
open).

The alternative icon has the same name as the original one, with a
"+rtl" string appended to the lfun string. The alternative icon is
only active if the file is found. The icon themes `default', `oxygen'
and `classic' have been updated accordingly.

Fixes bug #4451.

lib/Makefile.am

@@ -568,7 +568,9 @@ dist_images_DATA1X = \
 	images/tab-group-close.svgz \
 	images/copy.svgz \
 	images/cut.svgz \
+	images/depth-decrement+rtl.svgz \
 	images/depth-decrement.svgz \
+	images/depth-increment+rtl.svgz \
 	images/depth-increment.svgz \
 	images/dialog-preferences.svgz \
 	images/dialog-show-new-inset_citation.svgz \
@@ -609,13 +611,18 @@ dist_images_DATA1X = \
 	images/layout.svgz \
 	images/layout-document.svgz \
 	images/layout-paragraph.svgz \
+	images/layout-toggle_Chunk.svgz \
+	images/layout-toggle_Description+rtl.svgz \
 	images/layout-toggle_Description.svgz \
+	images/layout-toggle_Enumerate+rtl.svgz  \
 	images/layout-toggle_Enumerate.svgz  \
+	images/layout-toggle_Itemize+rtl.svgz \
 	images/layout-toggle_Itemize.svgz \
+	images/layout-toggle_Labeling+rtl.svgz \
 	images/layout-toggle_Labeling.svgz \
 	images/layout-toggle_List.svgz \
 	images/layout-toggle_LyX-Code.svgz \
-	images/layout-toggle_Chunk.svgz \
+	images/layout-toggle_Section+rtl.svgz \
 	images/layout-toggle_Section.svgz \
 	images/lyxfiles-system.svgz \
 	images/lyxfiles-user.svgz \
@@ -1853,7 +1860,9 @@ dist_imagesoxygen_DATA1X = \
 	images/oxygen/closetab.svgz \
 	images/oxygen/copy.svgz \
 	images/oxygen/cut.svgz \
+	images/oxygen/depth-decrement+rtl.svgz \
 	images/oxygen/depth-decrement.svgz \
+	images/oxygen/depth-increment+rtl.svgz \
 	images/oxygen/depth-increment.svgz \
 	images/oxygen/dialog-preferences.svgz \
 	images/oxygen/dialog-show-new-inset_citation.svgz \
@@ -1892,12 +1901,17 @@ dist_imagesoxygen_DATA1X = \
 	images/oxygen/layout-paragraph.svgz \
 	images/oxygen/layout.svgz \
 	images/oxygen/layout-toggle_Chunk.svgz \
+	images/oxygen/layout-toggle_Description+rtl.svgz \
 	images/oxygen/layout-toggle_Description.svgz \
-	images/oxygen/layout-toggle_Enumerate.svgz \
+	images/oxygen/layout-toggle_Enumerate+rtl.svgz  \
+	images/oxygen/layout-toggle_Enumerate.svgz  \
+	images/oxygen/layout-toggle_Itemize+rtl.svgz \
 	images/oxygen/layout-toggle_Itemize.svgz \
+	images/oxygen/layout-toggle_Labeling+rtl.svgz \
 	images/oxygen/layout-toggle_Labeling.svgz \
 	images/oxygen/layout-toggle_List.svgz \
 	images/oxygen/layout-toggle_LyX-Code.svgz \
+	images/oxygen/layout-toggle_Section+rtl.svgz \
 	images/oxygen/layout-toggle_Section.svgz \
 	images/oxygen/lyx-quit.svgz \
 	images/oxygen/marginalnote-insert.svgz \
@@ -2057,7 +2071,9 @@ dist_imagesclassic_DATA = \
 	images/classic/tab-group-close.png  \
 	images/classic/copy.png  \
 	images/classic/cut.png  \
+	images/classic/depth-decrement+rtl.png  \
 	images/classic/depth-decrement.png  \
+	images/classic/depth-increment+rtl.png  \
 	images/classic/depth-increment.png  \
 	images/classic/dialog-preferences.png  \
 	images/classic/dialog-show_mathdelimiter.png  \
@@ -2092,14 +2108,19 @@ dist_imagesclassic_DATA = \
 	images/classic/layout-document.png  \
 	images/classic/layout-paragraph.png  \
 	images/classic/layout.png  \
-	images/classic/layout-toggle_Description.png  \
-	images/classic/layout-toggle_Enumerate.png  \
-	images/classic/layout-toggle_Itemize.png  \
-	images/classic/layout-toggle_Labeling.png  \
-	images/classic/layout-toggle_List.png  \
-	images/classic/layout-toggle_LyX-Code.png  \
 	images/classic/layout-toggle_Chunk.png  \
-	images/classic/layout-toggle_Section.png  \
+	images/classic/layout-toggle_Description+rtl.png \
+	images/classic/layout-toggle_Description.png \
+	images/classic/layout-toggle_Enumerate+rtl.png  \
+	images/classic/layout-toggle_Enumerate.png  \
+	images/classic/layout-toggle_Itemize+rtl.png \
+	images/classic/layout-toggle_Itemize.png \
+	images/classic/layout-toggle_Labeling+rtl.png \
+	images/classic/layout-toggle_Labeling.png \
+	images/classic/layout-toggle_List.png \
+	images/classic/layout-toggle_LyX-Code.png \
+	images/classic/layout-toggle_Section+rtl.png \
+	images/classic/layout-toggle_Section.png \
 	images/classic/marginalnote-insert.png  \
 	images/classic/master-buffer-update.png  \
 	images/classic/master-buffer-view.png  \

lib/ui/stdtoolbars.inc

@@ -36,13 +36,18 @@
 #      Item "Small font" "font-size small"
 #      Item Emphasized set-emph
 #
+#   BidiItem is like Item, but an alternative icon (with name ending
+#   with "+rtl") will be used <hen the paragraph has a right-to-left
+#   layout.
+#
 #   Layouts adds the layouts combo-box to the toolbar
 #
 #   Separator adds some spacing to the toolbar
 #
-#   Minibuffer adds the command buffer (Qt only, only one may exist)
+#   Minibuffer adds the command buffer (only one may exist)
 #
-#   TableInsert "The tooltip" adds a special widget for quick insertion of tables
+#   TableInsert "The tooltip" adds a special widget for quick
+#   insertion of tables
 #
 #   PopupMenu "name" "The tooltip"
 #
@@ -113,12 +118,12 @@ ToolbarSet
 
 	Toolbar "extra" "Extra"
 		Item "Default" "layout"
-		Item "Numbered list" "layout-toggle Enumerate"
-		Item "Itemized list" "layout-toggle Itemize"
-		Item "Labeled List" "layout-toggle Labeling"
-		Item "Description" "layout-toggle Description"
-		Item "Increase depth" "depth-increment"
-		Item "Decrease depth" "depth-decrement"
+		BidiItem "Numbered list" "layout-toggle Enumerate"
+		BidiItem "Itemized list" "layout-toggle Itemize"
+		BidiItem "Labeled List" "layout-toggle Labeling"
+		BidiItem "Description" "layout-toggle Description"
+		BidiItem "Increase depth" "depth-increment"
+		BidiItem "Decrease depth" "depth-decrement"
 		Separator
 		Item "Insert figure float" "float-insert figure"
 		Item "Insert table float" "float-insert table"

src/frontends/qt/Action.cpp

@@ -11,6 +11,7 @@
 #include <config.h>
 
 #include "Action.h"
+#include "GuiApplication.h"
 
 // DispatchResult.h is needed by the windows compiler because lyx::dispatch
 // returns a DispatchResult const reference. Gcc does not complain. Weird...
@@ -33,27 +34,25 @@ namespace frontend {
 
 Action::Action(FuncRequest func, QIcon const & icon, QString const & text,
                QString const & tooltip, QObject * parent)
-	: QAction(parent), func_(make_shared<FuncRequest>(move(func)))
+	: QAction(parent), func_(make_shared<FuncRequest>(move(func))), icon_(icon)
 {
-	init(icon, text, tooltip);
+	init(text, tooltip);
 }
 
 
 Action::Action(shared_ptr<FuncRequest const> func,
                QIcon const & icon, QString const & text,
                QString const & tooltip, QObject * parent)
-	: QAction(parent), func_(func)
+	: QAction(parent), func_(func), icon_(icon)
 {
-	init(icon, text, tooltip);
+	init(text, tooltip);
 }
 
 
-void Action::init(QIcon const & icon, QString const & text,
-                  QString const & tooltip)
+void Action::init(QString const & text, QString const & tooltip)
 {
 	// only Qt/Mac handles that
 	setMenuRole(NoRole);
-	setIcon(icon);
 	setText(text);
 	setToolTip(tooltip);
 	setStatusTip(tooltip);
@@ -76,6 +75,11 @@ void Action::update()
 		setCheckable(false);
 	}
 
+	if (rtlIcon_.isNull() || !guiApp->rtlContext())
+		setIcon(icon_);
+	else
+		setIcon(rtlIcon_);
+
 	setEnabled(status.enabled());
 }
 

src/frontends/qt/Action.h

@@ -43,6 +43,8 @@ public:
 	       QIcon const & icon, QString const & text,
 	       QString const & tooltip, QObject * parent);
 
+	void setRtlIcon(QIcon const & icon) { rtlIcon_ = icon; }
+
 	void update();
 
 Q_SIGNALS:
@@ -53,8 +55,10 @@ private Q_SLOTS:
 	void action();
 
 private:
-	void init(QIcon const & icon, QString const & text, QString const & tooltip);
+	void init(QString const & text, QString const & tooltip);
 	std::shared_ptr<FuncRequest const> func_;
+	QIcon icon_;
+	QIcon rtlIcon_;
 };
 
 

src/frontends/qt/GuiApplication.cpp

@@ -486,7 +486,7 @@ QString themeIconName(QString const & action)
 }
 
 
-QString iconName(FuncRequest const & f, bool unknown)
+QString iconName(FuncRequest const & f, bool unknown, QString const & suffix)
 {
 	initializeResources();
 	QString name1;
@@ -527,6 +527,9 @@ QString iconName(FuncRequest const & f, bool unknown)
 		}
 	}
 
+	// maybe a suffix?
+	name1 += suffix;
+
 	QStringList imagedirs;
 	imagedirs << "images/" << "images/ipa/";
 	search_mode mode = theGuiApp()->imageSearchMode();
@@ -611,7 +614,7 @@ QPixmap getPixmap(QString const & path, QString const & name, QString const & ex
 }
 
 
-QIcon getIcon(FuncRequest const & f, bool unknown)
+QIcon getIcon(FuncRequest const & f, bool unknown, QString const & suffix)
 {
 #if (QT_VERSION >= 0x040600)
 	if (lyxrc.use_system_theme_icons) {
@@ -628,7 +631,7 @@ QIcon getIcon(FuncRequest const & f, bool unknown)
 	}
 #endif
 
-	QString icon = iconName(f, unknown);
+	QString icon = iconName(f, unknown, suffix);
 	if (icon.isEmpty())
 		return QIcon();
 
@@ -2400,6 +2403,16 @@ void GuiApplication::resetGui()
 }
 
 
+bool GuiApplication::rtlContext() const
+{
+	if (current_view_ && current_view_->currentBufferView()) {
+		BufferView const * bv = current_view_->currentBufferView();
+		return bv->cursor().innerParagraph().isRTL(bv->buffer().params());
+	} else
+		return layoutDirection() == Qt::RightToLeft;
+}
+
+
 void GuiApplication::createView(int view_id)
 {
 	createView(QString(), true, view_id);

src/frontends/qt/GuiApplication.h

@@ -88,6 +88,8 @@ public:
 	void hideDialogs(std::string const & name, Inset * inset) const;
 	///
 	void resetGui();
+	/// Return true if current position is RTL of if no document is open and interface if RTL
+	bool rtlContext() const;
 
 	///
 	Clipboard & clipboard();
@@ -256,7 +258,8 @@ private:
 extern GuiApplication * guiApp;
 
 /// \return the icon file name for the given action.
-QString iconName(FuncRequest const & f, bool unknown);
+QString iconName(FuncRequest const & f, bool unknown,
+                 QString const & suffix = QString());
 
 /// \return the pixmap for the given path, name and extension.
 /// in case of errors a warning is produced and an empty pixmap is returned.
@@ -267,7 +270,8 @@ QPixmap getPixmap(QString const & path, QString const & name, QString const & ex
 bool getPixmap(QPixmap & pixmap, QString const & path);
 
 /// \return an icon for the given action.
-QIcon getIcon(FuncRequest const & f, bool unknown);
+QIcon getIcon(FuncRequest const & f, bool unknown,
+			  QString const & suffix = QString());
 
 ///
 GuiApplication * theGuiApp();

src/frontends/qt/GuiToolbar.cpp

@@ -129,7 +129,10 @@ Action * GuiToolbar::addItem(ToolbarItem const & item)
 		text += " [" + toqstr(bindings.begin()->print(KeySequence::ForGui)) + "]";
 
 	Action * act = new Action(item.func_, getIcon(*item.func_, false), text,
-	                          text, this);
+							  text, this);
+	if (item.type_ == ToolbarItem::BIDICOMMAND)
+		act->setRtlIcon(getIcon(*item.func_, false, "+rtl"));
+
 	actions_.append(act);
 	return act;
 }
@@ -518,6 +521,11 @@ void GuiToolbar::add(ToolbarItem const & item)
 			LYXERR0("Unknown dynamic menu type: " << item.name_);
 		break;
 	}
+	case ToolbarItem::BIDICOMMAND: {
+		if (!getStatus(*item.func_).unknown())
+			addAction(addItem(item));
+		break;
+		}
 	case ToolbarItem::COMMAND: {
 		if (!getStatus(*item.func_).unknown())
 			addAction(addItem(item));

src/frontends/qt/Toolbars.cpp

@@ -63,6 +63,7 @@ ToolbarInfo & ToolbarInfo::read(Lexer & lex)
 {
 	enum {
 		TO_COMMAND = 1,
+		TO_BIDICOMMAND,
 		TO_ENDTOOLBAR,
 		TO_SEPARATOR,
 		TO_LAYOUTS,
@@ -79,6 +80,7 @@ ToolbarInfo & ToolbarInfo::read(Lexer & lex)
 	};
 
 	struct LexerKeyword toolTags[] = {
+		{ "bidiitem", TO_BIDICOMMAND},
 		{ "dynamicmenu", TO_DYNAMICMENU},
 		{ "end", TO_ENDTOOLBAR },
 		{ "exportformats", TO_EXPORTFORMATS },
@@ -138,6 +140,20 @@ ToolbarInfo & ToolbarInfo::read(Lexer & lex)
 			}
 			break;
 
+		case TO_BIDICOMMAND:
+			if (lex.next(true)) {
+				docstring const tooltip = translateIfPossible(lex.getDocString());
+				lex.next(true);
+				string const func_arg = lex.getString();
+				LYXERR(Debug::PARSER, "ToolbarInfo::read TO_BIDICOMMAND func: `"
+					<< func_arg << '\'');
+
+				FuncRequest func =
+					lyxaction.lookupFunc(func_arg);
+				add(ToolbarItem(ToolbarItem::BIDICOMMAND, func, tooltip));
+			}
+			break;
+
 		case TO_MINIBUFFER:
 			add(ToolbarItem(ToolbarItem::MINIBUFFER,
 				FuncRequest(FuncCode(ToolbarItem::MINIBUFFER))));

src/frontends/qt/Toolbars.h

@@ -29,6 +29,8 @@ namespace frontend {
 class ToolbarItem {
 public:
 	enum Type {
+		/// command/action with rtl version
+		BIDICOMMAND,
 		/// command/action
 		COMMAND,
 		/// the command buffer