View Issue Details

IDProjectCategoryView StatusLast Update
0017449ScribusGeneralpublic2025-03-10 22:01
Reporterale Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status newResolutionopen 
Product Version1.7.1.svn 
Summary0017449: [PATCH] In the clipboard, refactor the detection of bold and italics
DescriptionScClipboardProcessor has three copies of the same code for detecting the bold and italic styles in the font.

Since it would be useful to use the same code also when importing from Markdown and for shortcuts, I'd like to suggest to refactor the code into three functions.
TagsNo tags attached.
PatchYes

Activities

ale

2025-03-09 20:36

manager   ~0052193

The patch attached:

- adds four functions in SCFonts
- the functions are used in ScClipboardProcessor
- the functions are similar to the original code in ScClipboardProcessor
- fixes the bug hidden in the detection of the regular style (the "style" variable was shadowed by a local variable in the if condition)
- adds the detection for additional terms typical for the styles
- adds a cache of the styles that have been detected for each family:
  - the cache is built when the style is used
  - in the future, i'd like to use those functions for ctrl-shift-b and ctrl-shift-i shortcuts
  - in the future, i'd like to add more advanced heuristic for detecting bold and italics (user defined in the document setup)
bold-italic-refactoring.diff (8,096 bytes)   
From 9264be225efc40dbf3ff97cd0381d2ed44e82e05 Mon Sep 17 00:00:00 2001
From: ale rimoldi <ale@graphicslab.org>
Date: Sun, 9 Mar 2025 21:34:49 +0100
Subject: refactor the detection of the font bold and italic styles in their
 own functions


diff --git a/scribus/scclipboardprocessor.cpp b/scribus/scclipboardprocessor.cpp
index 46275d110..7a65dc213 100644
--- a/scribus/scclipboardprocessor.cpp
+++ b/scribus/scclipboardprocessor.cpp
@@ -418,27 +418,16 @@ void ScClipboardProcessor::html_MSFT_ParseParagraphs(xmlNode *node, QMap<QString
 				QString currFamily(segment.family.isEmpty() ? currPstyle.charStyle().font().family() : segment.family);
 				if (!segment.isBold && !segment.isItalic)
 				{
-					QStringList styles = availableFonts.fontMap[currFamily];
-					QString style;
-					if (!styles.isEmpty())
-						style = styles[0];
-					if (styles.contains("Regular"))
-						style = "Regular";
-					else if (styles.contains("Roman"))
-						style = "Roman";
-					else if (styles.contains("Medium"))
-						style = "Medium";
-					else if (styles.contains("Book"))
-						style = "Book";
+					style = availableFonts.getRegularStyle(currFamily);
 				}
 				else
 				{
-					if(segment.isBold && !segment.isItalic)
-						style = "Bold";
-					if (!segment.isBold && segment.isItalic)
-						style = "Italic";
-					if(segment.isBold && segment.isItalic)
-						style = "Bold Italic";
+					if (segment.isBold && !segment.isItalic)
+						style = availableFonts.getBoldStyle(currFamily);
+					else if (!segment.isBold && segment.isItalic)
+						style = availableFonts.getItalicStyle(currFamily);
+					else if (segment.isBold && segment.isItalic)
+						style = availableFonts.getBoldItalicStyle(currFamily);
 				}
 				if (segment.hasUnderline)
 				{
@@ -686,27 +675,16 @@ void ScClipboardProcessor::html_LibreOffice_ParseParagraphs(xmlNode *node, QMap<
 				QString currFamily(segment.family.isEmpty() ? currPstyle.charStyle().font().family() : segment.family);
 				if (!segment.isBold && !segment.isItalic)
 				{
-					QStringList styles = availableFonts.fontMap[currFamily];
-					QString style;
-					if (!styles.isEmpty())
-						style = styles[0];
-					if (styles.contains("Regular"))
-						style = "Regular";
-					else if (styles.contains("Roman"))
-						style = "Roman";
-					else if (styles.contains("Medium"))
-						style = "Medium";
-					else if (styles.contains("Book"))
-						style = "Book";
+					style = availableFonts.getRegularStyle(currFamily);
 				}
 				else
 				{
-					if(segment.isBold && !segment.isItalic)
-						style = "Bold";
-					if (!segment.isBold && segment.isItalic)
-						style = "Italic";
-					if(segment.isBold && segment.isItalic)
-						style = "Bold Italic";
+					if (segment.isBold && !segment.isItalic)
+						style = availableFonts.getBoldStyle(currFamily);
+					else if (!segment.isBold && segment.isItalic)
+						style = availableFonts.getItalicStyle(currFamily);
+					else if (segment.isBold && segment.isItalic)
+						style = availableFonts.getBoldItalicStyle(currFamily);
 				}
 				if (segment.hasUnderline)
 				{
@@ -974,27 +952,16 @@ void ScClipboardProcessor::html_Cocoa_ParseParagraphs(xmlNode *node, QMap<QStrin
 				QString currFamily(currPstyle.charStyle().font().family());
 				if (!segment.isBold && !segment.isItalic)
 				{
-					QStringList styles = availableFonts.fontMap[currFamily];
-					QString style;
-					if (!styles.isEmpty())
-						style = styles[0];
-					if (styles.contains("Regular"))
-						style = "Regular";
-					else if (styles.contains("Roman"))
-						style = "Roman";
-					else if (styles.contains("Medium"))
-						style = "Medium";
-					else if (styles.contains("Book"))
-						style = "Book";
+					style = availableFonts.getRegularStyle(currFamily);
 				}
 				else
 				{
-					if(segment.isBold && !segment.isItalic)
-						style = "Bold";
-					if (!segment.isBold && segment.isItalic)
-						style = "Italic";
-					if(segment.isBold && segment.isItalic)
-						style = "Bold Italic";
+					if (segment.isBold && !segment.isItalic)
+						style = availableFonts.getBoldStyle(currFamily);
+					else if (!segment.isBold && segment.isItalic)
+						style = availableFonts.getItalicStyle(currFamily);
+					else if (segment.isBold && segment.isItalic)
+						style = availableFonts.getBoldItalicStyle(currFamily);
 				}
 				if (segment.hasUnderline)
 				{
diff --git a/scribus/scfonts.cpp b/scribus/scfonts.cpp
index 43349ce52..bacbe3c41 100644
--- a/scribus/scfonts.cpp
+++ b/scribus/scfonts.cpp
@@ -1355,6 +1355,98 @@ void SCFonts::getFonts(const QString& pf, bool showFontInfo)
 	writeFontCache(pf);
 }
 
+QString SCFonts::getRegularStyle(const QString& family)
+{
+	if (!styleCache.contains(family))
+	{
+		styleCache[family] = {};
+	}
+	if (!styleCache[family].contains(StyleType::Regular))
+	{
+		QString style;
+		QStringList styles = fontMap[family];
+		if (!styles.isEmpty())
+			style = styles[0];
+		if (styles.contains("Regular"))
+			style = "Regular";
+		else if (styles.contains("Roman"))
+			style = "Roman";
+		else if (styles.contains("Medium"))
+			style = "Medium";
+		else if (styles.contains("Book"))
+			style = "Book";
+		styleCache[family][StyleType::Regular] = style;
+	}
+	return styleCache[family][StyleType::Regular];
+}
+
+/*
+ * TODO:
+ * - how to transition:
+ *   - from "Condensed" to "Condensed Bold"?
+ *   - from "Light" to "Regular"?
+ *   - from "Bold" to "Black"?
+ */
+QString SCFonts::getBoldStyle(const QString& family)
+{
+	if (!styleCache.contains(family))
+	{
+		styleCache[family] = {};
+	}
+	if (!styleCache[family].contains(StyleType::Bold))
+	{
+		QString style;
+		QStringList styles = fontMap[family];
+		if (styles.contains("Bold"))
+			style = "Bold";
+		else if (styles.contains("Black"))
+			style = "Black";
+		styleCache[family][StyleType::Bold] = style;
+	}
+	return styleCache[family][StyleType::Bold];
+}
+
+QString SCFonts::getBoldItalicStyle(const QString& family)
+{
+	if (!styleCache.contains(family))
+	{
+		styleCache[family] = {};
+	}
+	if (!styleCache[family].contains(StyleType::BoldItalic))
+	{
+		QString style;
+		QString bold = getBoldStyle(family);
+		QString italic = getItalicStyle(family);
+		if (bold == "")
+			style = italic;
+		else if  (italic == "")
+			style = bold;
+		else
+			style = bold + " " + italic;
+		styleCache[family][StyleType::BoldItalic] = style;
+	}
+	return styleCache[family][StyleType::BoldItalic];
+}
+
+QString SCFonts::getItalicStyle(const QString& family)
+{
+	if (!styleCache.contains(family))
+	{
+		styleCache[family] = {};
+	}
+	if (!styleCache[family].contains(StyleType::Italic))
+	{
+		QString style;
+		QStringList styles = fontMap[family];
+		if (styles.contains("Italic"))
+			style = "Italic";
+		else if (styles.contains("Oblique"))
+			style = "Oblique";
+		styleCache[family][StyleType::Italic] = style;
+	}
+	return styleCache[family][StyleType::Italic];
+}
+
 void SCFonts::addRejectedFont(const QString& fontPath, const QString& message)
 {
 	rejectedFonts.insert(fontPath, message);
diff --git a/scribus/scfonts.h b/scribus/scfonts.h
index dce78e900..22e16e7bd 100644
--- a/scribus/scfonts.h
+++ b/scribus/scfonts.h
@@ -41,6 +41,14 @@ class SCRIBUS_API SCFonts : public QMap<QString,ScFace>
 		SCFonts();
 		~SCFonts() = default;
 
+		enum class StyleType
+		{
+			Regular,
+			Bold,
+			Italic,
+			BoldItalic
+		};
+
 		void updateFontMap();
 		void getFonts(const QString& pf, bool showFontInfo = false);
 		ScFace loadScalableFont(const QString &filename);
@@ -61,6 +69,14 @@ class SCRIBUS_API SCFonts : public QMap<QString,ScFace>
 		QMap<QString, QStringList> fontMap;
 		QMap<QString, QString> rejectedFonts;
 
+		/// maps regular, bold, itacs style for each family
+		QMap<QString, QMap<StyleType, QString>> styleCache;
+
+		QString getRegularStyle(const QString& family);
+		QString getBoldStyle(const QString& family);
+		QString getBoldItalicStyle(const QString& family);
+		QString getItalicStyle(const QString& family);
+
 	private:
 		void readFontCache(const QString& pf);
 		void writeFontCache(const QString& pf) const;
bold-italic-refactoring.diff (8,096 bytes)   

cbradney

2025-03-10 22:01

administrator   ~0052201

I fixed the duplicate style variable. Will review the rest tomorrow. Thanks

Issue History

Date Modified Username Field Change
2025-03-09 20:27 ale New Issue
2025-03-09 20:36 ale Note Added: 0052193
2025-03-09 20:36 ale File Added: bold-italic-refactoring.diff
2025-03-10 22:01 cbradney Note Added: 0052201