completion now prefers completing variable names before trying comp specs

improved color picking for error messages

src/builtin/arrops.rs

@@ -1,6 +1,9 @@
+use std::collections::VecDeque;
+
+use ariadne::Span;
 
 use crate::{
-  getopt::{Opt, OptSpec, get_opts_from_tokens}, jobs::JobBldr, libsh::error::{ShErr, ShErrKind, ShResult}, parse::{NdRule, Node}, prelude::*, procio::{IoStack, borrow_fd}, state::{self, VarFlags, VarKind, write_vars}
+  getopt::{Opt, OptSpec, get_opts_from_tokens}, jobs::JobBldr, libsh::error::{ShErr, ShErrKind, ShResult, ShResultExt}, parse::{NdRule, Node}, prelude::*, procio::{IoStack, borrow_fd}, state::{self, VarFlags, VarKind, read_vars, write_vars}
 };
 
 use super::setup_builtin;
@@ -102,15 +105,19 @@ fn arr_push_inner(node: Node, io_stack: &mut IoStack, job: &mut JobBldr, end: En
 		return Err(ShErr::at(ShErrKind::ExecFail, blame, "push: missing array name".to_string()));
 	};
 
-	for (val, _) in argv {
+	for (val, span) in argv {
 		let push_val = val.clone();
-		if let Err(e) = write_vars(|v| v.get_arr_mut(&name).map(|arr| match end {
-			End::Front => arr.push_front(push_val),
-			End::Back => arr.push_back(push_val),
-		})) {
-			state::set_status(1);
-			return Err(e);
-		};
+		write_vars(|v| {
+			if let Ok(arr) = v.get_arr_mut(&name) {
+				match end {
+					End::Front => arr.push_front(push_val),
+					End::Back => arr.push_back(push_val),
+				}
+				Ok(())
+			} else {
+				v.set_var(&name, VarKind::Arr(VecDeque::from([push_val])), VarFlags::NONE)
+			}
+		}).blame(span)?;
 	}
 
   state::set_status(0);

src/libsh/error.rs

@@ -13,7 +13,9 @@ use crate::{
 
 pub type ShResult<T> = Result<T, ShErr>;
 
-pub struct ColorRng;
+pub struct ColorRng {
+	last_color: Option<Color>,
+}
 
 impl ColorRng {
 	fn get_colors() -> &'static [Color] {
@@ -39,6 +41,16 @@ impl ColorRng {
 			Color::Fixed(105), // medium purple
 		]
 	}
+
+	pub fn last_color(&mut self) -> Color {
+		if let Some(color) = self.last_color.take() {
+			return color;
+		} else {
+			let color = self.next().unwrap_or(Color::White);
+			self.last_color = Some(color);
+			color
+		}
+	}
 }
 
 impl Iterator for ColorRng {
@@ -51,11 +63,19 @@ impl Iterator for ColorRng {
 }
 
 thread_local! {
-	static COLOR_RNG: RefCell<ColorRng> = const { RefCell::new(ColorRng) };
+	static COLOR_RNG: RefCell<ColorRng> = const { RefCell::new(ColorRng { last_color: None }) };
 }
 
 pub fn next_color() -> Color {
-	COLOR_RNG.with(|rng| rng.borrow_mut().next().unwrap())
+	COLOR_RNG.with(|rng| {
+		let color = rng.borrow_mut().next().unwrap();
+		rng.borrow_mut().last_color = Some(color);
+		color
+	})
+}
+
+pub fn last_color() -> Color {
+	COLOR_RNG.with(|rng| rng.borrow_mut().last_color())
 }
 
 pub trait ShResultExt {
@@ -145,14 +165,14 @@ impl ShErr {
 		Self { kind, src_span: None, labels: vec![], sources: vec![], notes: vec![msg.into()] }
 	}
 	pub fn at(kind: ShErrKind, span: Span, msg: impl Into<String>) -> Self {
-		let color = next_color();
+		let color = last_color(); // use last_color to ensure the same color is used for the label and the message given
 		let src = span.span_source().clone();
 		let msg: String = msg.into();
 		Self::new(kind, span.clone())
 			.with_label(src, ariadne::Label::new(span).with_color(color).with_message(msg))
 	}
 	pub fn labeled(self, span: Span, msg: impl Into<String>) -> Self {
-		let color = next_color();
+		let color = last_color();
 		let src = span.span_source().clone();
 		let msg: String = msg.into();
 		self.with_label(src, ariadne::Label::new(span).with_color(color).with_message(msg))
@@ -230,7 +250,7 @@ impl ShErr {
 	}
 	pub fn print_error(&self) {
 		let default = || {
-			eprintln!("{}", self.kind);
+			eprintln!("\n{}", self.kind);
 			for note in &self.notes {
 				eprintln!("note: {note}");
 			}
@@ -245,6 +265,7 @@ impl ShErr {
 				.cloned()
 				.ok_or_else(|| format!("Failed to fetch source '{}'", src.name()))
 		});
+		eprintln!();
 		if report.eprint(cache).is_err() {
 			default();
 		}

src/parse/execute.rs

@@ -315,6 +315,7 @@ impl Dispatcher {
     Ok(())
   }
   fn exec_subsh(&mut self, subsh: Node) -> ShResult<()> {
+		let blame = subsh.get_span().clone();
     let NdRule::Command { assignments, argv } = subsh.class else {
       unreachable!()
     };
@@ -328,7 +329,7 @@ impl Dispatcher {
       let mut argv = match prepare_argv(argv) {
         Ok(argv) => argv,
         Err(e) => {
-          e.print_error();
+          e.try_blame(blame).print_error();
           return;
         }
       };
@@ -376,7 +377,7 @@ impl Dispatcher {
 
 		blame.rename(func_name.clone());
 
-    let argv = prepare_argv(argv)?;
+    let argv = prepare_argv(argv).try_blame(blame.clone())?;
     let result = if let Some(ref mut func_body) = read_logic(|l| l.get_func(&func_name)) {
       let _guard = ScopeGuard::exclusive_scope(Some(argv));
 			func_body.body_mut().propagate_context(func_ctx);
@@ -833,6 +834,7 @@ impl Dispatcher {
 		}
   }
   fn exec_cmd(&mut self, cmd: Node) -> ShResult<()> {
+		let blame = cmd.get_span().clone();
     let context = cmd.context.clone();
     let NdRule::Command { assignments, argv } = cmd.class else {
       unreachable!()
@@ -856,7 +858,7 @@ impl Dispatcher {
 
     self.io_stack.append_to_frame(cmd.redirs);
 
-    let exec_args = ExecArgs::new(argv)?;
+    let exec_args = ExecArgs::new(argv).blame(blame)?;
     let _guard = self.io_stack.pop_frame().redirect()?;
     let job = self.job_stack.curr_job_mut().unwrap();
 

src/parse/mod.rs

@@ -7,7 +7,7 @@ use lex::{LexFlags, LexStream, Span, SpanSource, Tk, TkFlags, TkRule};
 
 use crate::{
   libsh::{
-    error::{ShErr, ShErrKind, ShResult, next_color},
+    error::{ShErr, ShErrKind, ShResult, last_color, next_color},
     utils::{NodeVecUtils, TkVecUtils},
   },
   prelude::*,
@@ -135,7 +135,7 @@ impl Node {
     }
   }
 	pub fn get_context(&self, msg: String) -> (SpanSource, Label<Span>) {
-		let color = next_color();
+		let color = last_color();
 		let span = self.get_span().clone();
 		(
 			span.clone().source().clone(),
@@ -1745,7 +1745,7 @@ pub fn get_redir_file(class: RedirType, path: PathBuf) -> ShResult<File> {
 }
 
 fn parse_err_full(reason: &str, blame: &Span, context: LabelCtx) -> ShErr {
-	let color = next_color();
+	let color = last_color();
 	ShErr::new(ShErrKind::ParseErr, blame.clone())
 		.with_label(
 			blame.span_source().clone(),

src/readline/complete.rs

@@ -433,6 +433,13 @@ impl CompSpec for BashCompSpec {
     if self.function.is_some() {
       candidates.extend(self.exec_comp_func(ctx)?);
     }
+		candidates = candidates.into_iter()
+			.map(|c| {
+				let stripped = c.strip_prefix(&expanded).unwrap_or_default();
+				format!("{prefix}{stripped}")
+			})
+		.collect();
+
 		candidates.sort_by_key(|c| c.len()); // sort by length to prioritize shorter completions, ties are then sorted alphabetically
 
     Ok(candidates)
@@ -766,8 +773,25 @@ impl Completer {
       self.token_span = (cursor_pos, cursor_pos);
     }
 
-    // Try programmable completion first
+    // Use marker-based context detection for sub-token awareness (e.g. VAR_SUB
+    // inside a token). Run this before comp specs so variable completions take
+    // priority over programmable completion.
+    let (mut marker_ctx, token_start) = self.get_completion_context(&line, cursor_pos);
 
+    if marker_ctx.last() == Some(&markers::VAR_SUB) {
+      if let Some(cur) = ctx.words.get(ctx.cword) {
+        self.token_span.0 = token_start;
+        let mut span = cur.span.clone();
+        span.set_range(token_start..self.token_span.1);
+        let raw_tk = span.as_str();
+        let candidates = complete_vars(raw_tk);
+        if !candidates.is_empty() {
+          return Ok(CompResult::from_candidates(candidates));
+        }
+      }
+    }
+
+    // Try programmable completion
     match self.try_comp_spec(&ctx)? {
       CompSpecResult::NoMatch { flags } => {
         if flags.contains(CompOptFlags::DIRNAMES) {
@@ -801,9 +825,6 @@ impl Completer {
 
     self.token_span = (cur_token.span.range().start, cur_token.span.range().end);
 
-    // Use marker-based context detection for sub-token awareness (e.g. VAR_SUB
-    // inside a token)
-    let (mut marker_ctx, token_start) = self.get_completion_context(&line, cursor_pos);
     self.token_span.0 = token_start;
     cur_token
       .span
@@ -828,12 +849,9 @@ impl Completer {
       _ if self.dirs_only => complete_dirs(&expanded),
       Some(markers::COMMAND) => complete_commands(&expanded),
       Some(markers::VAR_SUB) => {
-        let var_candidates = complete_vars(&raw_tk);
-        if var_candidates.is_empty() {
-          complete_filename(&expanded)
-        } else {
-          var_candidates
-        }
+        // Variable completion already tried above and had no matches,
+        // fall through to filename completion
+        complete_filename(&expanded)
       }
       Some(markers::ARG) => complete_filename(&expanded),
       _ => complete_filename(&expanded),

src/readline/mod.rs

@@ -341,7 +341,7 @@ impl ShedVi {
 
         match self.completer.complete(line, cursor_pos, direction) {
 					Err(e) => {
-						self.writer.flush_write(&format!("\n{e}\n\n"))?;
+						e.print_error();
 
 						// Printing the error invalidates the layout
 						self.old_layout = None;

src/readline/term.rs

@@ -780,7 +780,7 @@ impl AsFd for TermReader {
   }
 }
 
-#[derive(Debug)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct Layout {
   pub prompt_end: Pos,
   pub cursor: Pos,

src/state.rs

@@ -467,9 +467,7 @@ thread_local! {
 
 /// A shell function
 ///
-/// Consists of the BraceGrp Node and the stored ParsedSrc that the node refers
-/// to. The Node must be stored with the ParsedSrc because the tokens of the
-/// node contain an Arc<String> Which refers to the String held in ParsedSrc
+/// Wraps the BraceGrp Node that forms the body of the function, and provides some helper methods to extract it from the parse tree
 #[derive(Clone, Debug)]
 pub struct ShFunc(Node);