search

Kronuz / ColorHighlight

🎨 Lightweight Color Highlight colorizer for Sublime Text
MIT License
118 stars 12 forks source link

Fix detection of tmTheme theme formats. #6

Closed spacesuitdiver closed 5 years ago

spacesuitdiver commented 5 years ago

This should fix #4.

spacesuitdiver commented 5 years ago

@Kronuz what's the scenario where we need to match against XML doctype versus an actual tag that would (probably) more accurately represent the contents of the file? Is <plist version="1.0"> used only with later variants of the themes? I surfed around the themes and couldn't find an instance where this tag was not used but admittedly not exhaustively. I'll gladly make the change I just think maybe we are coding around a case that doesn't exist.

spacesuitdiver commented 5 years ago

I did change so it would match the entire document for <plist version="1.0">, I missed the carrot in the Regex last night :)

Kronuz commented 5 years ago

The thing is we’re not searching in the entire content, only at the beginning; and plists (being XMLs) can begin with any of those: <?xml ..., <!DOCTYPE or <plist alll we care to know at that point is it’s a XML so we procede to try to load as plist.

spacesuitdiver commented 5 years ago

I updated the PR to search the document for the plist.

Kronuz commented 5 years ago

Searching for <plist version=“1.0”> in the entire document would be less efficient, and we really don’t need to; also, you’d need to change to use search() instead of match() for it to work.

Having said that I’d much rather matching the three at the beginning instead, as I suggested.

spacesuitdiver commented 5 years ago

Made the change 👍