Duplicat colorsheme in status bar when user has custom colorscheme with similar the same name

[dustdfg]

Description of the problem or steps to reproduce

1. Ctrl+e
2. > set colorscheme + Tab (to see all the schemes in status line)
3. See two duplicates

You can select any of them and will load user's custom implementation. If you just manually type set colorscheme <scheme>, it will also select users color scheme

Specifications

Commit hash: v2.0.13 68d88b5
OS: Debian 12
Terminal: foot

[dustdfg]

Note: I almost created PR so I will soon create it or upload the patch in this issue if I will have troubles

[dustdfg]

OK. I have a solution but is partial and breaks other things. The commit showed by github above this message de-duplicates solves this problem but make worse another problem with reload #3059. If the commit is applied, the reload will create two duplicates instead of one (except files that have redefinition in user config dir :/)

[JoeKar]

From my perspective user defined color schemes should take precedence in the moment they have the same name. So the easiest way is to add them first and while adding the builtin we should check if there is already an entry with the same name and thus skip the current. In #3059 I confirmed the bug.

[dustdfg]

0_o current implementation rtfiles.go#L164 already loads user defined colorschemes at first and only than load builtins but the function that adds builtins doesn't check user defined colorscheme was already loaded. My commit do exactly it: adds check if user defined function was already loaded

[JoeKar]

Correct me if I'm wrong, but your commit adds it as long as the asset file name is != to the current checked user defined one.
Furthermore it doesn't respect the pattern.

How about the following more generic approach?

func AddRuntimeFilesFromAssets(fileType RTFiletype, directory, pattern string) {
	files, err := rt.AssetDir(directory)
	if err != nil {
		return
	}

assetLoop:
	for _, f := range files {
		if ok, _ := path.Match(pattern, f); ok {
			for _, rf := range realFiles[fileType] {
				if f == rf.Name() + strings.TrimPrefix(pattern, "*") {
					continue assetLoop
				}
			}
			AddRuntimeFile(fileType, assetFile(path.Join(directory, f)))
		}
	}
}

[dustdfg]

Correct me if I'm wrong, but your commit adds it as long as the asset file name is != to the current checked user defined one.

Yes my code is a piece of s**t spaghetti :) Approach without negation of check is better and more convenient and easy to understand

Yes. It adds the embedded file only if it's name not equal to the name of user loaded file. It is your aim not to load duplicates. Your approach is to skip file when the names are equal. Your code won't skip when names different. My code won't add when filenames are equal. It is in general the same thing but yes as stated above approach without negation is more convenient and easy to understand

Furthermore it doesn't respect the pattern.

You are absolutely right

How about the following more generic approach?

It is much better than my. No spaghetti. I think it is ready for PR :)

It even avoids the pitfall of the check if len(realFiles[fileType]) > 0 { if doesn't have any realFile. Non existence of which caused a bug: when you don't have realFile, you won't load any embedded file of appropriate file type (in my non generic case only color schemes).

And doesn't have any influence to reload which will just create 1 duplicate as expected...

Thank you

[dustdfg]

I also found an issue #2795 where the problem was described before this issue so it also should be closed after merging the PR