mirror of
https://github.com/nushell/nushell.git
synced 2025-08-27 06:52:48 +02:00
Fix highlighting of aliases to external commands (#15408)
Fixes #12965. # Description The syntax highlighter currently can't tell the difference between external command calls and _alias_ calls to external commands. This leads to unexpected behavior when `highlight_resolved_externals` is enabled. Specifically, when this option is enabled, alias calls aliasing external commands are highlighted according to `color_config.shape_external_resolved` if and only if a command exists in the PATH directories whose name happens to be the same as the name of the alias. The syntax highlighter also can't tell the difference between internal command calls and alias calls aliasing internal commands, but this just results in these alias calls being highlighted like normal internal command calls, which isn't too unexpected. It is ambiguous what the correct behavior should be. The three options I can think of are: 1. Treat all alias calls (both aliasing internal and external commands) as a new shape, like `shape_alias`, and highlight accordingly. 2. Highlight all alias calls like internal command calls, including those aliasing external commands. 3. Highlight alias calls aliasing external commands according to how the aliased command would have been highlighted. I personally like 3. and it was easy to implement (unless I missed something), so that's what this PR does right now. I'll be happy to implement a different solution if appropriate. Discussion welcome :) # User-Facing Changes All changes are in syntax highlighting only. Behavior _before_ this PR:  Behavior _after_ this PR:  # Tests + Formatting I didn't write any tests because I couldn't find any existing tests for syntax highlighting.
This commit is contained in:
@@ -23,6 +23,22 @@ impl Highlighter for NuHighlighter {
|
||||
}
|
||||
}
|
||||
|
||||
// <<<<<<< HEAD
|
||||
// =======
|
||||
// let config = self.stack.get_config(&self.engine_state);
|
||||
// let highlight_resolved_externals = config.highlight_resolved_externals;
|
||||
// let mut working_set = StateWorkingSet::new(&self.engine_state);
|
||||
// let block = parse(&mut working_set, None, line.as_bytes(), false);
|
||||
// let (shapes, global_span_offset) = {
|
||||
// let mut shapes = flatten_block(&working_set, &block);
|
||||
// // Highlighting externals has a config point because of concerns that using which to resolve
|
||||
// // externals may slow down things too much.
|
||||
// if highlight_resolved_externals {
|
||||
// for (span, shape) in shapes.iter_mut() {
|
||||
// if let FlatShape::External(aliased_command_span) = *shape {
|
||||
// let str_contents = working_set.get_span_contents(aliased_command_span);
|
||||
// >>>>>>> df798b657 (Fix highlighting of aliases to external commands)
|
||||
|
||||
/// Result of a syntax highlight operation
|
||||
#[derive(Default)]
|
||||
pub(crate) struct HighlightResult {
|
||||
@@ -50,10 +66,8 @@ pub(crate) fn highlight_syntax(
|
||||
// externals may slow down things too much.
|
||||
if highlight_resolved_externals {
|
||||
for (span, shape) in shapes.iter_mut() {
|
||||
if *shape == FlatShape::External {
|
||||
let str_contents =
|
||||
working_set.get_span_contents(Span::new(span.start, span.end));
|
||||
|
||||
if let FlatShape::External(aliased_command_span) = shape {
|
||||
let str_contents = working_set.get_span_contents(**aliased_command_span);
|
||||
let str_word = String::from_utf8_lossy(str_contents).to_string();
|
||||
let paths = env::path_str(engine_state, stack, *span).ok();
|
||||
let res = if let Ok(cwd) = engine_state.cwd(Some(stack)) {
|
||||
@@ -126,7 +140,7 @@ pub(crate) fn highlight_syntax(
|
||||
FlatShape::Float => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::Range => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::InternalCall(_) => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::External => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::External(_) => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::ExternalArg => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::ExternalResolved => add_colored_token(&shape.1, next_token),
|
||||
FlatShape::Keyword => add_colored_token(&shape.1, next_token),
|
||||
|
@@ -1,5 +1,5 @@
|
||||
use nu_protocol::{
|
||||
DeclId, Span, SyntaxShape, VarId,
|
||||
DeclId, GetSpan, Span, SyntaxShape, VarId,
|
||||
ast::{
|
||||
Argument, Block, Expr, Expression, ExternalArgument, ImportPatternMember, ListItem,
|
||||
MatchPattern, PathMember, Pattern, Pipeline, PipelineElement, PipelineRedirection,
|
||||
@@ -18,7 +18,10 @@ pub enum FlatShape {
|
||||
Custom(DeclId),
|
||||
DateTime,
|
||||
Directory,
|
||||
External,
|
||||
// The stored span contains the name of the called external command:
|
||||
// This is only different from the span containing the call's head if this
|
||||
// call is through an alias, and is only useful for its contents (not its location).
|
||||
External(Box<Span>),
|
||||
ExternalArg,
|
||||
ExternalResolved,
|
||||
Filepath,
|
||||
@@ -58,7 +61,7 @@ impl FlatShape {
|
||||
FlatShape::Custom(_) => "shape_custom",
|
||||
FlatShape::DateTime => "shape_datetime",
|
||||
FlatShape::Directory => "shape_directory",
|
||||
FlatShape::External => "shape_external",
|
||||
FlatShape::External(_) => "shape_external",
|
||||
FlatShape::ExternalArg => "shape_externalarg",
|
||||
FlatShape::ExternalResolved => "shape_external_resolved",
|
||||
FlatShape::Filepath => "shape_filepath",
|
||||
@@ -321,7 +324,16 @@ fn flatten_expression_into(
|
||||
}
|
||||
Expr::ExternalCall(head, args) => {
|
||||
if let Expr::String(..) | Expr::GlobPattern(..) = &head.expr {
|
||||
output.push((head.span, FlatShape::External));
|
||||
output.push((
|
||||
head.span,
|
||||
// If this external call is through an alias, then head.span contains the
|
||||
// name of the alias (needed to highlight the right thing), but we also need
|
||||
// the name of the aliased command (to decide *how* to highlight the call).
|
||||
// The parser actually created this head by cloning from the alias's definition
|
||||
// and then just overwriting the `span` field - but `span_id` still points to
|
||||
// the original span, so we can recover it from there.
|
||||
FlatShape::External(Box::new(working_set.get_span(head.span_id))),
|
||||
));
|
||||
} else {
|
||||
flatten_expression_into(working_set, head, output);
|
||||
}
|
||||
|
Reference in New Issue
Block a user