View Issue Details

IDProjectCategoryView StatusLast Update
0016217ScribusInternalpublic2020-08-19 06:03
Reporterale Assigned To 
PrioritynormalSeverityfeatureReproducibilityN/A
Status newResolutionopen 
Product Version1.5.6.svn 
Summary0016217: [PATCH] introduce an enum for the tab types
Descriptionthe attached patch adds an enum class for the tab types.

the fact that i could not find the differences between 2 and 3 is a sign that it might be useful to avoid using the ints for the tab types.

if this patch gets accepted, i can think of using it for TabRecord::tabType (introducing an enum class in there will trigger a few changes... so i prefer doing it after i know that the team agrees that an enum is a good thing here... and in the meantime i can use it for the hruler...)
TagsNo tags attached.
PatchYes

Activities

ale

2020-08-18 14:37

manager   ~0047959

please feel free to pick better names and to define what 2 is...
TabType.diff (419 bytes)   
diff --git a/scribus/styles/paragraphstyle.h b/scribus/styles/paragraphstyle.h
index 75cafbfa0..e5bfe0b80 100644
--- a/scribus/styles/paragraphstyle.h
+++ b/scribus/styles/paragraphstyle.h
@@ -56,6 +56,13 @@ public:
 		ManualHyphenation    = 1,
 		AutomaticHyphenation = 2
 	};
+	enum class TabType {
+		Left = 0,
+		Right = 1,
+		Unused = 2,
+		Dot = 3,
+		Center = 4
+	};
 	struct TabRecord
 	{
 		qreal tabPosition;
TabType.diff (419 bytes)   

ale

2020-08-18 15:09

manager   ~0047960

ok, probably 2 is period and 3 is comma... says the UI...

jghali

2020-08-18 16:14

administrator   ~0047961

Last edited: 2020-08-18 16:15

Fyi there is already an enum for tabs in pageitem_textframe.cpp. It's called TabStatus.

ale

2020-08-18 16:37

manager   ~0047962

which is only used inside of a function and since it's in a cpp file i fear that it cannot be used in another unit.

i would suggest to have a TabType as a "stand alone" enum class or as a subtype of another class.
i don't really care where it is, as long as it can be used and it is used.
(i would avoid a scribustypes.h thing, though...)

jghali

2020-08-18 16:39

administrator   ~0047964

>> which is only used inside of a function and since it's in a cpp file i fear that it cannot be used in another unit.

Nothing prevents you to move it out the cpp file if appropriate.

jghali

2020-08-18 17:03

administrator   ~0047965

Looking at the TabStatus enum, it seems to already match the UI for paragraph styles in style manager, so it should be rather moved out of pageitem_textframe.cpp and potentially renamed.

ale

2020-08-19 06:03

manager   ~0047977

i would also use an enum class to limit the visibility of the names (and "simplify" them) and "help" the programmers use the enum instead of the integers (documentation of the code + avoiding subtle bugs)...

if you have a suggestion for a new place for the enum, i can do the work...
if i recall correctly styles/paragraphstyle.h would be a good place...

Issue History

Date Modified Username Field Change
2020-08-18 14:36 ale New Issue
2020-08-18 14:36 ale Summary introduce an enum for the tab types => [PATCH] introduce an enum for the tab types
2020-08-18 14:36 ale Patch No => Yes
2020-08-18 14:37 ale File Added: TabType.diff
2020-08-18 14:37 ale Note Added: 0047959
2020-08-18 15:09 ale Note Added: 0047960
2020-08-18 16:14 jghali Note Added: 0047961
2020-08-18 16:15 jghali Note Edited: 0047961
2020-08-18 16:18 jghali Category - => Internal
2020-08-18 16:37 ale Note Added: 0047962
2020-08-18 16:39 jghali Note Added: 0047964
2020-08-18 17:03 jghali Note Added: 0047965
2020-08-19 06:03 ale Note Added: 0047977