Allow case insensitive search for .tmtheme paths

[gjtorikian]

This was an annoying bug to unravel. 😓 On case insensitive systems, like macOS, tmtheme ought to be just as valid as tmTheme.

[gjtorikian]

Not at all sure why CI would fail here.

[gjtorikian]

Is this project still maintained?

[Enselic]

Several people are watching project activity as far I know. The main reason no action has been taken on this PR is probably that CI does not pass. At least that's the main reason for me personally.

[keith-hall]

Indeed, I wanted to help investigate the CI failures but forgot and haven't had time.

I would probably also expect it to detect the filesystem case sensitivity and act accordingly, not just always assume insensitive is okay, otherwise this change could break it for others, no?

[gjtorikian]

The main reason no action has been taken on this PR is probably that CI does not pass. At least that's the main reason for me personally.

Sorry, this comment was meant to ask for help on that. My previous PR with CI passing (#466) also hasn't been merged, so I wasn't sure if anyone was looking here.

I would probably also expect it to detect the filesystem case sensitivity and act accordingly, not just always assume insensitive is okay

There are two other locations in the existing code where case insensitivity is being used:

syntect/src/parsing/syntax_set.rs

Lines 186 to 200 in c61ce60

	self.syntaxes.iter().rev().find(|&s| s.file_extensions.iter().any(|e| e.eq_ignore_ascii_case(extension)))
	}
	/// Searches for a syntax first by extension and then by case-insensitive name
	///
	/// This is useful for things like Github-flavoured-markdown code block highlighting where all
	/// you have to go on is a short token given by the user
	pub fn find_syntax_by_token<'a>(&'a self, s: &str) -> Option<&'a SyntaxReference> {
	{
	let ext_res = self.find_syntax_by_extension(s);
	if ext_res.is_some() {
	return ext_res;
	}
	}
	self.syntaxes.iter().rev().find(|&syntax| syntax.name.eq_ignore_ascii_case(s))

In other words, this is the only code location where case sensitivity is (incorrectly, imho) required.

[keith-hall]

sublime-syntax files are looked for case sensitively: https://github.com/trishume/syntect/blob/master/src/parsing/syntax_set.rs#L490

As for file extensions when looking up a syntax, it has been proven that Sublime Text does this in a case insensitive manner, and looking up a syntax for an arbitrary token is just a convenience that syntect offers, and it makes sense to be insensitive.

So if we can prove that Sublime also handles varying case tmTheme and sublime-syntax files on a case-sensitive filesystem, then it would make sense to add a test for it and then, yes, merge it. Or if it handles it only on a case-insensitive filesystem, then we should do the same in syntect. But until then, I feel we could be breaking Sublime compatibility with this change...

[keith-hall]

I was able to reproduce this failing test locally. It seems it is because there is a folder called InspiredGitHub.tmtheme. I suggest we change the code to ensure we only look for files.

[keith-hall]

And for what it's worth, I tested Sublime Text 4148 on Linux (i.e. case-sensitive file system) and .Sublime-Syntax files are not loaded, and .tmtheme files don't show up in the color scheme picker UI. So we just need to prove how it works on a case-insensitive file system to decide the way forward here.

[gjtorikian]

So we just need to prove how it works on a case-insensitive file system to decide the way forward here.

I am not sure if this is a task for me or the repo's administrators, but, Sublime 4143 on macOS (case-insensitive) will happily load a file whether it's .tmTheme or .tmtheme.

[keith-hall]

Cool, so let's fix it to only look for files and you will get my approval :)

[gjtorikian]

Done!

Edit: ah it seems VS Code formatted the file. Happy to undo that if desired.

[Enselic]

Thank you for the contribution.

Indeed it is mostly formatting changes. For the record, I am strongly in favor of syntect running cargo fmt as part of CI. That way the problem of PRs containging pure formatting changes goes away completely. It is a very common practice in the Rust ecosystem (and other ecosystems) in my experience, and it works great. It is a time saver both for maintainers and contributors.