Scan for external color schemes and combine with the built-in ones #293

kenmo-pb · 2024-12-03T06:17:09Z

Here's an IDE feature I've wanted for a while...

In the Preferences window, we're given a list of Default Color Schemes, which come hard-coded in a DataSection.

We are able to distribute custom color schemes as *.prefs files, but it's a cumbersome process to use Preferences > Import Settings > Browse > Open > Include Color Settings > Import Settings to change between them, especially if you use 2 or 3 often or at least light & dark.

This PR adds the functionality to scan for external color scheme files, combine them with the built-in default themes, and present them all in the Preferences window dropdown list for easy scheme switching.

You can test with some of the *.prefs color schemes I have shared here or here. Drop them in your PB themes subfolder.

Some info:

rather than iterate through the hard-coded DataSection to build the dropdown list, the color schemes are now a dynamic list (of a new ColorSchemeStruct containing an array of color values)
the built-in default schemes remain unchanged (list initialized from that DataSection)
if custom scheme files are found, they are merged into the list, and sorted alphabetically (I think sorting is good if you have many custom schemes listed!)
the "Accessibility" scheme retains its special position at the bottom of the list!
if no custom scheme files are found, the list stays unchanged. identical to the current IDE.
because this all adds a decent amount of code related to color schemes, I've created a new ColorSchemes.pb file. this actually reduces much code elsewhere in other files!
currently, I scan PureBasicPath$ + #DEFAULT_ThemePath (the themes subfolder) just like scanning for custom icon themes. perhaps this should scan a different/new subfolder?
it expects *.prefs extension, but will also accept a few others
if color entries are missing, this assumes a "best guess" based on the background/foreground color
I think scanning for schemes once, at IDE startup, is acceptable. an alternative might be to scan every time Preferences is opened.

With this basis, we could add more features such as:

true "instant" scheme switching by keyboard shortcut or menu item - without using the Preferences window - I already have a version of this implemented, for a future PR...
assigning a pair of color schemes to automatically follow the system's Light or Dark mode status
backup/restore color scheme when toggling Accessibility mode (some users in the forum have complained about losing their color settings after enabling then disabling Accessibility mode)

t-harter

I think a bit of rework is needed to bring this code more in line more with the IDE code conventions (String variable naming, use of braces, etc.). Otherwise, good work.

Could you also add a small section documenting this to the manual? (Documentation/English/Reference/ide_preferences.txt)

t-harter · 2024-12-15T18:06:02Z

PureBasicIDE/ColorSchemes.pb

+
+
+; These (from Preferences.pb) are used
+Global PreferenceToolsPanelFrontColor, PreferenceToolsPanelBackColor


Variables that are needed in multiple files should be moved to Common.pb. In a similar way, procedure declarations used outside of one file go in Declarations.pb

t-harter · 2024-12-15T18:07:30Z

PureBasicIDE/ColorSchemes.pb

+  Protected Result.i = #True
+
+  For i = 0 To #COLOR_Last
+    If ((i <> #COLOR_Selection) And (i <> #COLOR_SelectionFront)) ; selection colors may follow OS, so skip them for scheme match check


Please remove unneeded braces. Only add them when there are multiple And/Or statement to avoid ambiguity. Especially when there is only one statement, adding braces only reduces readability.

Will do. In this highlighted example, would you remove just the outer parentheses, or all the parentheses?

I'd remove all of them. Only use parentheses when they make a difference. The only exception to this is when And/Or are mixed in a statement because PB has a bit weird precedence rules here, so I tend to add them in this case just so it is clear to everybody what is happening.

t-harter · 2024-12-15T18:08:22Z

PureBasicIDE/ColorSchemes.pb

+
+
+; Returns #True if the specified color scheme matches the user's current color settings, otherwise #False
+Procedure.i ColorSchemeMatchesCurrentSettings(*ColorScheme.ColorSchemeStruct)


Integer is the default type. So adding it to procedure declarations is unneeded. The same goes for variable declarations.

The general style in the IDE sources is:

Integer variables without any type declaration

String variables always in the Variable$ style

Float or double variables always with type (even outside of the declaration)

t-harter · 2024-12-15T18:09:18Z

PureBasicIDE/ColorSchemes.pb

+
+; Color Scheme structure
+Structure ColorSchemeStruct
+  Name.s


Please use the Name$ style to declare string variables as it is done in the rest of the IDE sources.

t-harter · 2024-12-15T18:12:49Z

PureBasicIDE/ColorSchemes.pb

+    ; Basic validation of color scheme file...
+    If (OpenPreferences(File))
+      Name.s = GetFilePart(File, #PB_FileSystem_NoExtension)
+      If (#True);(PreferenceGroup("Sections") And (ReadPreferenceLong("IncludeColors", 0) = 1))


What is the purpose of this?

Are you asking about the If (#True) ? I was doing some basic prefs validation there, but then I bypassed it to make the code more lenient in what it accepts as a color scheme file. The comment just shows what logic was formerly there. I can remove it completely or re-enable it.

So either remove it or uncomment the original check. Leaving it as it is does not really make sense in the long term.

t-harter · 2024-12-15T18:14:12Z

PureBasicIDE/ColorSchemes.pb

+          EndIf
+
+          If (*ColorScheme)
+            With *ColorScheme


For better readability, the With statement should be avoided, especially in larger code blocks.

t-harter · 2024-12-15T18:19:09Z

PureBasicIDE/ColorSchemes.pb

+          If (*ColorScheme) ; struct already specified - part of InitColorSchemes() list
+            ; Intentionally overwrite schemes of existing names - allows you to tweak the default themes, if desired
+            RemoveColorSchemeIfExists(Name)
+          Else ; NULL --> dynamically allocate a struct now - NOT part of InitColorSchemes() list!


What is the use of this code? I only see LoadColorSchemeFromFile() being called with a valid pointer as input, so this code should not be needed.

Good detective work 🕵️ The option to load a scheme file SEPARATE from the initialization list is needed for another feature built off this fork, which I've already implemented and I'm using, but I don't know if it would be accepted into the official IDE. (The feature is I can define an External Tool as scheme://<file> to instantly change to an arbitrary color scheme. scheme://<name> also works if it's in the scanned list.) Is that OK or should I delete it from this PR?

That change should be part of the other PR then, if and when it happens. Introducing code that might be used at some point in the future is a good way to generate dead code. I especially don't like this when it comes to pointer based code, because here correctness is especially important (bugs lead to a crash or memory leak easily). So we should focus only on code that is actually needed.

t-harter · 2024-12-15T18:21:26Z

PureBasicIDE/ColorSchemes.pb

+        If (DirectoryEntryType(Dir) = #PB_DirectoryEntry_File)
+          File.s = PureBasicPath$ + #DEFAULT_ThemePath + #PS$ + DirectoryEntryName(Dir)
+          Select (LCase(GetExtensionPart(File)))
+            Case "prefs", "scheme", "theme", "ini" ; only attempt to load certain file extensions


Why allow so many different file extensions? We define the format for this, so we can also define what extension is to be used. In my opinion, prefs alone is fine.

Again just trying to follow the "strict in what output you generate, but lenient in what input you accept" style. Yes it could only accept prefs. I thought at least one other extension (scheme ?) would be nice so users could differentiate standalone color scheme files from true "IDE preferences" files.

t-harter · 2024-12-15T18:31:15Z

currently, I scan PureBasicPath$ + #DEFAULT_ThemePath (the themes subfolder) just like scanning for custom icon themes. perhaps this should scan a different/new subfolder?

I think a separate folder makes more sense. It may also be a good idea to move most of the existing themes out of the data sections and put them there as files as well:

This will serve as an easy example/template for people to create their own, even based on the existing ones (just by copying the file)
I think the current logic to override an existing builtin scheme through a file with the same name is a bit confusing. Better make it explicit by just editing the file directly in the PB folder.
People can remove themes if they do not want them

I would leave just one scheme as builtin without a file (maybe the standard PB one). This ensures that the IDE always works even if somebody removes all the files. It is done the same way with the Icon themes as well. Maybe the "Accessibility" one should be treated separately as well.

kenmo-pb · 2024-12-17T04:56:46Z

Thanks for all the thoughtful feedback.

currently, I scan PureBasicPath$ + #DEFAULT_ThemePath (the themes subfolder) just like scanning for custom icon themes. perhaps this should scan a different/new subfolder?

I think a separate folder makes more sense. It may also be a good idea to move most of the existing themes out of the data sections and put them there as files as well:
* This will serve as an easy example/template for people to create their own, even based on the existing ones (just by copying the file)

* I think the current logic to override an existing builtin scheme through a file with the same name is a bit confusing. Better make it explicit by just editing the file directly in the PB folder.

* People can remove themes if they do not want them
I would leave just one scheme as builtin without a file (maybe the standard PB one). This ensures that the IDE always works even if somebody removes all the files. It is done the same way with the Icon themes as well. Maybe the "Accessibility" one should be treated separately as well.

If you're OK with it, sure we can move the default color schemes into external files which the user could modify or delete. I'll just leave the PureBasic / SpiderBasic theme in the DataSection. Should the subfolder be Schemes/schemes to match Themes/themes ?

t-harter · 2024-12-17T20:30:23Z

Should the subfolder be Schemes/schemes to match Themes/themes ?

The word "Schemes" alone is too generic in my opinion and could mean many things. I think I'd call it "ColorSchemes" / "colorschemes", or maybe just "Colors" if you prefer it short.

kenmo-pb · 2024-12-18T04:27:35Z

I vote for subfolder name ColorSchemes, I think it is immediately understandable to someone who sees it, more than either of those words on their own.

alphasnd · 2024-12-18T15:37:04Z

If we plan to add more subdirs, it should be done in a 'IDE' dir or something, because it will mix up with other PB dirs

kenmo-pb · 2024-12-19T03:47:36Z

If we make an IDE subfolder, would we move the Themes into there too? Then should the IDE look for Themes in both old and new location? It gets complicated...

I don't mind submitting "behind the scenes" IDE features, but restructuring subfolders seems like a bigger decision you should decide, not me... 🙂 I'll update this PR with whatever you prefer.

alphasnd · 2024-12-19T07:40:55Z

Well, just go for the colorscheme subfolder, we will refactor it if there is more

… the built-in ones

…her than 'Themes'

…clarations.pb

…ture feature

…s files

…eme files

kenmo-pb · 2025-01-08T04:56:23Z

Hi @t-harter @alphasnd , I pushed an update that hopefully addresses all of your feedback. They're small individual commits for easy review, but I can squash it all down again if everything looks good.

Two caveats:

I added a note about color scheme files to ide_preferences.txt but only English. Also I've never edited the doc files before, I hope the format is right.
⚠️ The makefile(s) will also need an update, to create the ColorSchemes subfolder and copy in all DefaultColorSchemes prefs. Similar to how it handles Themes. I hope you or Fred can make this update, because I am inexperienced with makefile syntax and what I should write.

fantaisie-software · 2025-01-17T09:41:47Z

Looks good, thanks !

t-harter requested changes Dec 15, 2024

View reviewed changes

kenmo-pb added 10 commits January 7, 2025 08:52

Add ColorSchemes.pb, scan for external color schemes and combine with…

dcaeea4

… the built-in ones

scan-for-color-schemes: Do not use With/EndWith

e965219

scan-for-color-schemes: Scan a dedicated 'ColorSchemes' subfolder rat…

775106d

…her than 'Themes'

scan-for-color-schemes: Re-enable prefs section validation check

793e958

scan-for-color-schemes: Only accept ".prefs" file extension

c5fa294

scan-for-color-schemes: Move Globals and Declares to Common.pb and De…

4a6489d

…clarations.pb

scan-for-color-schemes: Follow the PBIDE source code standards

742bc17

scan-for-color-schemes: Remove structure allocation intended for a fu…

0b496b4

…ture feature

scan-for-color-schemes: Move (8) embedded schemes into external .pref…

66a2ace

…s files

scan-for-color-schemes: Documentation note about adding new color sch…

c5e255f

…eme files

kenmo-pb force-pushed the scan-for-color-schemes--squashed branch from ecb1e72 to c5e255f Compare January 8, 2025 04:44

fantaisie-software merged commit 16898e6 into fantaisie-software:devel Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scan for external color schemes and combine with the built-in ones #293

Scan for external color schemes and combine with the built-in ones #293

kenmo-pb commented Dec 3, 2024

t-harter left a comment

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter Dec 17, 2024

t-harter Dec 15, 2024

t-harter Dec 15, 2024

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter Dec 17, 2024

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter Dec 17, 2024

t-harter Dec 15, 2024

kenmo-pb Dec 17, 2024

t-harter commented Dec 15, 2024

kenmo-pb commented Dec 17, 2024

t-harter commented Dec 17, 2024

kenmo-pb commented Dec 18, 2024

alphasnd commented Dec 18, 2024

kenmo-pb commented Dec 19, 2024

alphasnd commented Dec 19, 2024

kenmo-pb commented Jan 8, 2025

fantaisie-software commented Jan 17, 2025



		; These (from Preferences.pb) are used
		Global PreferenceToolsPanelFrontColor, PreferenceToolsPanelBackColor



		; Returns #True if the specified color scheme matches the user's current color settings, otherwise #False
		Procedure.i ColorSchemeMatchesCurrentSettings(*ColorScheme.ColorSchemeStruct)

Scan for external color schemes and combine with the built-in ones #293

Scan for external color schemes and combine with the built-in ones #293

Conversation

kenmo-pb commented Dec 3, 2024

t-harter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t-harter commented Dec 15, 2024

kenmo-pb commented Dec 17, 2024

t-harter commented Dec 17, 2024

kenmo-pb commented Dec 18, 2024

alphasnd commented Dec 18, 2024

kenmo-pb commented Dec 19, 2024

alphasnd commented Dec 19, 2024

kenmo-pb commented Jan 8, 2025

fantaisie-software commented Jan 17, 2025