7fe4fdf · bat · git.iko.soy

git.iko.soy/bat

Introduce MapExtensionToUnknown MappingTarget (#1889)

7fe4fdfcbolgiano5 years ago

CHANGELOG.md+3

@@ -9,6 +9,7 @@

10- Python syntax highlighting no longer suffers from abysmal performance in specific scenarios. See #1688 (@keith-hall)

11- First line not shown in diff context. See #1891 (@divagant-martian)

12+- Do not ignore syntaxes that handle file names with a `*.conf` extension. See #1703 (@cbolgiano)

13## Performance

14## Performance

@@ -41,6 +42,8 @@

41- Deprecate `HighlightingAssets::syntaxes()` and `HighlightingAssets::syntax_for_file_name()`. Use `HighlightingAssets::get_syntaxes()` and `HighlightingAssets::get_syntax_for_path()` instead. They return a `Result` which is needed for upcoming lazy-loading work to improve startup performance. They also return which `SyntaxSet` the returned `SyntaxReference` belongs to. See #1747, #1755, #1776, #1862 (@Enselic)

42- Deprecate `HighlightingAssets::syntaxes()` and `HighlightingAssets::syntax_for_file_name()`. Use `HighlightingAssets::get_syntaxes()` and `HighlightingAssets::get_syntax_for_path()` instead. They return a `Result` which is needed for upcoming lazy-loading work to improve startup performance. They also return which `SyntaxSet` the returned `SyntaxReference` belongs to. See #1747, #1755, #1776, #1862 (@Enselic)

42- Remove `HighlightingAssets::from_files` and `HighlightingAssets::save_to_cache`. Instead of calling the former and then the latter you now make a single call to `bat::assets::build`. See #1802 (@Enselic)

43- Remove `HighlightingAssets::from_files` and `HighlightingAssets::save_to_cache`. Instead of calling the former and then the latter you now make a single call to `bat::assets::build`. See #1802 (@Enselic)

43- Replace the `error::Error(error::ErrorKind, _)` struct and enum with an `error::Error` enum. `Error(ErrorKind::UnknownSyntax, _)` becomes `Error::UnknownSyntax`, etc. Also remove the `error::ResultExt` trait. These changes stem from replacing `error-chain` with `thiserror`. See #1820 (@Enselic)

44- Replace the `error::Error(error::ErrorKind, _)` struct and enum with an `error::Error` enum. `Error(ErrorKind::UnknownSyntax, _)` becomes `Error::UnknownSyntax`, etc. Also remove the `error::ResultExt` trait. These changes stem from replacing `error-chain` with `thiserror`. See #1820 (@Enselic)

45+- Add new `MappingTarget` enum variant `MapExtensionToUnknown`. Refer to its docummentation for more information. Clients are adviced to treat `MapExtensionToUnknown` the same as `MapToUnknown` in exhaustive matches. See #1703 (@cbolgiano)

46+

src/assets.rs+78−18

@@ -153,9 +153,12 @@ impl HighlightingAssets {

153 }

154

155 /// Detect the syntax based on, in order:

156+ /// 1. Syntax mappings with [MappingTarget::MapTo] and [MappingTarget::MapToUnknown]

156− /// 1. Syntax mappings (e.g. `/etc/profile` -> `Bourne Again Shell (bash)`)

157+ /// (e.g. `/etc/profile` -> `Bourne Again Shell (bash)`)

157 /// 2. The file name (e.g. `Dockerfile`)

158 /// 2. The file name (e.g. `Dockerfile`)

159+ /// 3. Syntax mappings with [MappingTarget::MapExtensionToUnknown]

160+ /// (e.g. `*.conf`)

158− /// 3. The file name extension (e.g. `.rs`)

161+ /// 4. The file name extension (e.g. `.rs`)

159 ///

162 ///

160 /// When detecting syntax based on syntax mappings, the full path is taken

163 /// When detecting syntax based on syntax mappings, the full path is taken

161 /// into account. When detecting syntax based on file name, no regard is

164 /// into account. When detecting syntax based on file name, no regard is

@@ -165,9 +168,9 @@ impl HighlightingAssets {

165 ///

168 ///

166 /// Returns [Error::UndetectedSyntax] if it was not possible detect syntax

169 /// Returns [Error::UndetectedSyntax] if it was not possible detect syntax

167 /// based on path/file name/extension (or if the path was mapped to

170 /// based on path/file name/extension (or if the path was mapped to

171+ /// [MappingTarget::MapToUnknown] or [MappingTarget::MapExtensionToUnknown]).

168− /// [MappingTarget::MapToUnknown]). In this case it is appropriate to fall

172+ /// In this case it is appropriate to fall back to other methods to detect

169− /// back to other methods to detect syntax. Such as using the contents of

170− /// the first line of the file.

173+ /// syntax. Such as using the contents of the first line of the file.

171 ///

174 ///

172 /// Returns [Error::UnknownSyntax] if a syntax mapping exist, but the mapped

175 /// Returns [Error::UnknownSyntax] if a syntax mapping exist, but the mapped

173 /// syntax does not exist.

176 /// syntax does not exist.

@@ -177,20 +180,31 @@ impl HighlightingAssets {

177 mapping: &SyntaxMapping,

180 mapping: &SyntaxMapping,

178 ) -> Result<SyntaxReferenceInSet> {

181 ) -> Result<SyntaxReferenceInSet> {

179 let path = path.as_ref();

182 let path = path.as_ref();

180− match mapping.get_syntax_for(path) {

181− Some(MappingTarget::MapToUnknown) => {

182− Err(Error::UndetectedSyntax(path.to_string_lossy().into()))

183− }

184

183

184+ let syntax_match = mapping.get_syntax_for(path);

185+

186+ if let Some(MappingTarget::MapToUnknown) = syntax_match {

187+ return Err(Error::UndetectedSyntax(path.to_string_lossy().into()));

188+ }

189+

185− Some(MappingTarget::MapTo(syntax_name)) => self

190+ if let Some(MappingTarget::MapTo(syntax_name)) = syntax_match {

191+ return self

186 .find_syntax_by_name(syntax_name)?

192 .find_syntax_by_name(syntax_name)?

187− .ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned())),

193+ .ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned()));

194+ }

188

195

189− None => {

190− let file_name = path.file_name().unwrap_or_default();

196+ let file_name = path.file_name().unwrap_or_default();

191− self.get_extension_syntax(file_name)?

197+

198+ match (self.get_syntax_for_file_name(file_name)?, syntax_match) {

199+ (Some(syntax), _) => Ok(syntax),

200+

201+ (_, Some(MappingTarget::MapExtensionToUnknown)) => {

192− .ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into()))

202+ Err(Error::UndetectedSyntax(path.to_string_lossy().into()))

193 }

203 }

204+

205+ _ => self

206+ .get_syntax_for_file_extension(file_name)?

207+ .ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into())),

194 }

208 }

195 }

209 }

196

210

@@ -263,14 +277,24 @@ impl HighlightingAssets {

263 .map(|syntax| SyntaxReferenceInSet { syntax, syntax_set }))

277 .map(|syntax| SyntaxReferenceInSet { syntax, syntax_set }))

264 }

278 }

265

279

266− fn get_extension_syntax(&self, file_name: &OsStr) -> Result<Option<SyntaxReferenceInSet>> {

280+ fn get_syntax_for_file_name(&self, file_name: &OsStr) -> Result<Option<SyntaxReferenceInSet>> {

267 let mut syntax = self.find_syntax_by_extension(Some(file_name))?;

281 let mut syntax = self.find_syntax_by_extension(Some(file_name))?;

268 if syntax.is_none() {

282 if syntax.is_none() {

269− syntax = self.find_syntax_by_extension(Path::new(file_name).extension())?;

283+ syntax = try_with_stripped_suffix(file_name, |stripped_file_name| {

284+ self.get_syntax_for_file_name(stripped_file_name) // Note: recursion

285+ })?;

270 }

286 }

287+ Ok(syntax)

288+ }

289+

290+ fn get_syntax_for_file_extension(

291+ &self,

292+ file_name: &OsStr,

293+ ) -> Result<Option<SyntaxReferenceInSet>> {

294+ let mut syntax = self.find_syntax_by_extension(Path::new(file_name).extension())?;

271 if syntax.is_none() {

295 if syntax.is_none() {

272 syntax = try_with_stripped_suffix(file_name, |stripped_file_name| {

296 syntax = try_with_stripped_suffix(file_name, |stripped_file_name| {

273− self.get_extension_syntax(stripped_file_name) // Note: recursion

297+ self.get_syntax_for_file_extension(stripped_file_name) // Note: recursion

274 })?;

298 })?;

275 }

299 }

276 Ok(syntax)

300 Ok(syntax)

@@ -530,6 +554,42 @@ mod tests {

530 assert_eq!(test.syntax_for_file("test.h"), "C");

554 assert_eq!(test.syntax_for_file("test.h"), "C");

531 }

555 }

532

556

557+ #[test]

558+ fn syntax_detection_with_extension_mapping_to_unknown() {

559+ let mut test = SyntaxDetectionTest::new();

560+

561+ // Normally, a CMakeLists.txt file shall use the CMake syntax, even if it is

562+ // a bash script in disguise

563+ assert_eq!(

564+ test.syntax_for_file_with_content("CMakeLists.txt", "#!/bin/bash"),

565+ "CMake"

566+ );

567+

568+ // Other .txt files shall use the Plain Text syntax

569+ assert_eq!(

570+ test.syntax_for_file_with_content("some-other.txt", "#!/bin/bash"),

571+ "Plain Text"

572+ );

573+

574+ // If we setup MapExtensionToUnknown on *.txt, the match on the full

575+ // file name of "CMakeLists.txt" shall have higher prio, and CMake shall

576+ // still be used for it

577+ test.syntax_mapping

578+ .insert("*.txt", MappingTarget::MapExtensionToUnknown)

579+ .ok();

580+ assert_eq!(

581+ test.syntax_for_file_with_content("CMakeLists.txt", "#!/bin/bash"),

582+ "CMake"

583+ );

584+

585+ // However, for *other* files with a .txt extension, first-line fallback

586+ // shall now be used

587+ assert_eq!(

588+ test.syntax_for_file_with_content("some-other.txt", "#!/bin/bash"),

589+ "Bourne Again Shell (bash)"

590+ );

591+ }

592+

533 #[test]

593 #[test]

534 fn syntax_detection_is_case_sensitive() {

594 fn syntax_detection_is_case_sensitive() {

535 let mut test = SyntaxDetectionTest::new();

595 let mut test = SyntaxDetectionTest::new();

src/bin/bat/main.rs+1

@@ -72,6 +72,7 @@ fn get_syntax_mapping_to_paths<'a>(

72 for mapping in mappings {

73 match mapping {

74 (_, MappingTarget::MapToUnknown) => {}

75+ (_, MappingTarget::MapExtensionToUnknown) => {}

75 (matcher, MappingTarget::MapTo(s)) => {

76 (matcher, MappingTarget::MapTo(s)) => {

76 let globs = map.entry(*s).or_insert_with(Vec::new);

77 let globs = map.entry(*s).or_insert_with(Vec::new);

77 globs.push(matcher.glob().glob().into());

78 globs.push(matcher.glob().glob().into());

src/syntax_mapping.rs+14−6

@@ -6,8 +6,21 @@ use globset::{Candidate, GlobBuilder, GlobMatcher};

7#[derive(Debug, Clone, Copy, PartialEq)]

8pub enum MappingTarget<'a> {

9+ /// For mapping a path to a specific syntax.

9 MapTo(&'a str),

10 MapTo(&'a str),

11+

12+ /// For mapping a path (typically an extension-less file name) to an unknown

13+ /// syntax. This typically means later using the contents of the first line

14+ /// of the file to determine what syntax to use.

10 MapToUnknown,

15 MapToUnknown,

16+

17+ /// For mapping a file extension (e.g. `*.conf`) to an unknown syntax. This

18+ /// typically means later using the contents of the first line of the file

19+ /// to determine what syntax to use. However, if a syntax handles a file

20+ /// name that happens to have the given file extension (e.g. `resolv.conf`),

21+ /// then that association will have higher precedence, and the mapping will

22+ /// be ignored.

23+ MapExtensionToUnknown,

11}

24}

13#[derive(Debug, Clone, Default)]

26#[derive(Debug, Clone, Default)]

@@ -54,12 +67,7 @@ impl<'a> SyntaxMapping<'a> {

54 // Nginx and Apache syntax files both want to style all ".conf" files

67 // Nginx and Apache syntax files both want to style all ".conf" files

55 // see #1131 and #1137

68 // see #1131 and #1137

56 mapping

69 mapping

57− .insert("*.conf", MappingTarget::MapToUnknown)

70+ .insert("*.conf", MappingTarget::MapExtensionToUnknown)

58− .unwrap();

59−

60− // Re-insert a mapping for resolv.conf, see #1510

61− mapping

62− .insert("resolv.conf", MappingTarget::MapTo("resolv"))

63 .unwrap();

71 .unwrap();

65 for glob in &[

73 for glob in &[