-
Notifications
You must be signed in to change notification settings - Fork 6.2k
skip bash highlighting on Windows due to stack overflow crash #5981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| use ratatui::text::Line; | ||||||
|
|
||||||
| use ratatui::style::Style; | ||||||
| use ratatui::style::Stylize; | ||||||
| use ratatui::text::Line; | ||||||
| use ratatui::text::Span; | ||||||
| use std::sync::OnceLock; | ||||||
| use tree_sitter_highlight::Highlight; | ||||||
|
|
@@ -105,15 +106,31 @@ fn push_segment(lines: &mut Vec<Line<'static>>, segment: &str, style: Option<Sty | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn plain_text_lines(script: &str) -> Vec<Line<'static>> { | ||||||
| if script.is_empty() { | ||||||
| vec![Line::from("")] | ||||||
| } else { | ||||||
| script | ||||||
| .split('\n') | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| .map(|s| Line::from(s.to_string())) | ||||||
| .collect() | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// Convert a bash script into per-line styled content using tree-sitter's | ||||||
| /// bash highlight query. The highlighter is streamed so multi-line content is | ||||||
| /// split into `Line`s while preserving style boundaries. | ||||||
| pub(crate) fn highlight_bash_to_lines(script: &str) -> Vec<Line<'static>> { | ||||||
| // Runtime gate: on Windows, skip bash-specific highlighting and return plain text. | ||||||
| if cfg!(target_os = "windows") { | ||||||
| return plain_text_lines(script); | ||||||
| } | ||||||
|
Comment on lines
+124
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's no reason for this to be runtime, instead do #[cfg(not(target_os = "windows"))] on this function and add a different #[cfg(target_os = "windows")] impl that just calls plain_text_lines. |
||||||
|
|
||||||
| let mut highlighter = Highlighter::new(); | ||||||
| let iterator = | ||||||
| match highlighter.highlight(highlight_config(), script.as_bytes(), None, |_| None) { | ||||||
| Ok(iter) => iter, | ||||||
| Err(_) => return vec![script.to_string().into()], | ||||||
| Err(_) => return plain_text_lines(script), | ||||||
| }; | ||||||
|
|
||||||
| let mut lines: Vec<Line<'static>> = vec![Line::from("")]; | ||||||
|
|
@@ -132,7 +149,7 @@ pub(crate) fn highlight_bash_to_lines(script: &str) -> Vec<Line<'static>> { | |||||
| let style = highlight_stack.last().map(|h| highlight_for(*h).style()); | ||||||
| push_segment(&mut lines, &script[start..end], style); | ||||||
| } | ||||||
| Err(_) => return vec![script.to_string().into()], | ||||||
| Err(_) => return plain_text_lines(script), | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -145,10 +162,14 @@ pub(crate) fn highlight_bash_to_lines(script: &str) -> Vec<Line<'static>> { | |||||
|
|
||||||
| #[cfg(test)] | ||||||
| mod tests { | ||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| use super::*; | ||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| use pretty_assertions::assert_eq; | ||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| use ratatui::style::Modifier; | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| fn reconstructed(lines: &[Line<'static>]) -> String { | ||||||
| lines | ||||||
| .iter() | ||||||
|
|
@@ -162,6 +183,7 @@ mod tests { | |||||
| .join("\n") | ||||||
| } | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| fn dimmed_tokens(lines: &[Line<'static>]) -> Vec<String> { | ||||||
| lines | ||||||
| .iter() | ||||||
|
|
@@ -173,6 +195,7 @@ mod tests { | |||||
| .collect() | ||||||
| } | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| #[test] | ||||||
| fn dims_expected_bash_operators() { | ||||||
| let s = "echo foo && bar || baz | qux & (echo hi)"; | ||||||
|
|
@@ -185,6 +208,7 @@ mod tests { | |||||
| assert!(!dimmed.contains(&"echo".to_string())); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| #[test] | ||||||
| fn dims_redirects_and_strings() { | ||||||
| let s = "echo \"hi\" > out.txt; echo 'ok'"; | ||||||
|
|
@@ -197,6 +221,7 @@ mod tests { | |||||
| assert!(dimmed.contains(&"'ok'".to_string())); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| #[test] | ||||||
| fn highlights_command_and_strings() { | ||||||
| let s = "echo \"hi\""; | ||||||
|
|
@@ -219,6 +244,7 @@ mod tests { | |||||
| assert!(string_style.add_modifier.contains(Modifier::DIM)); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(not(target_os = "windows"))] | ||||||
| #[test] | ||||||
| fn highlights_heredoc_body_as_string() { | ||||||
| let s = "cat <<EOF\nheredoc body\nEOF"; | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spurious change