Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,10 @@ jobs:
if: matrix.os == 'ubuntu-latest'
- run: cargo clippy --all-targets --tests -- -D warnings
- run: cargo test
if: matrix.os == 'ubuntu-latest'
env:
TERM: linux
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently the TERM environment variable wasn't set at all in the Ubuntu CI environment so that was causing (per the --color docs) it to not print colors even when using eg --pretty which was causing tests to fail

So tried setting to what apparently is a pretty "generic" $TERM setting and it passed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems reasonable, but maybe that suggests that you should be able to control environment variables in tests?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya that could make sense

Here my specific understanding would be that then we'd have to eg "#[cfg(unix/windows)]-split" those particular tests (so that we could only set that environment variable for the Unix version), and would have to add that environment variable to every individual test that's using --pretty

To me that sounds clunkier vs putting it in the one place here that is basically saying "for all tests (on this platform) (by default) act like you're in this kind of underlying terminal environment"

- run: cargo test
if: matrix.os != 'ubuntu-latest'
- run: RUSTDOCFLAGS='--deny warnings' cargo doc --no-deps
if: matrix.os == 'ubuntu-latest'
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ bytecount = "0.6"
clap = { version = "4.3.0", features = ["derive", "wrap_help"] }
encoding_rs = "0.8.14"
encoding_rs_io = "0.1.6"
grep-cli = "0.1.8"
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another one of the ripgrep-derived crates

ignore = { package = "tree_sitter_grep_ignore", git = "https://github.com/helixbass/ripgrep", rev = "669ebd3", version = "0.4.20-dev.0" }
libc = "0.2.144"
libloading = "0.8.0"
Expand Down Expand Up @@ -73,6 +74,7 @@ assert_cmd = "2.0.11"
escargot = "0.5.7"
predicates = "3.0.3"
shlex = "1.1.0"
text-diff = "0.4.0"

[features]
default = ["bytecount/runtime-dispatch-simd"]
Expand Down
259 changes: 249 additions & 10 deletions src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ use std::{
sync::{Arc, Mutex},
};

use clap::{ArgGroup, Parser};
use clap::{ArgGroup, Parser, ValueEnum};
use ignore::{types::Types, WalkBuilder, WalkParallel};
use rayon::iter::IterBridge;
use termcolor::BufferWriter;
use termcolor::{BufferWriter, ColorChoice};

use crate::{
language::SupportedLanguage,
printer::StandardBuilder,
printer::{default_color_specs, ColorSpecs, StandardBuilder, UserColorSpec},
project_file_walker::{
get_project_file_walker_types, into_parallel_iterator, WalkParallelIterator,
},
Expand All @@ -19,6 +19,18 @@ use crate::{
NonFatalError,
};

#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)]
pub enum ColorChoiceArg {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost identical to the termcolor::ColorChoice that it ends up getting mapped to but I thought it made sense not to "conflate" those since (a) there is some mapping logic involved and (b) doing it this way we can tap into clap ValueEnum stuff

/// Colors will never be used.
Never,
/// The default. tree-sitter-grep tries to be smart.
Auto,
/// Colors will always be used regardless of where output is sent.
Always,
/// Like 'always', but emits ANSI escapes (even in a Windows console).
Ansi,
}

#[derive(Parser)]
#[clap(group(
ArgGroup::new("query_or_filter")
Expand Down Expand Up @@ -107,10 +119,137 @@ pub struct Args {
/// the offset of the matching part itself.
#[arg(short = 'b', long)]
pub byte_offset: bool,

/// This flag specifies color settings for use in the output.
///
/// This flag may be provided multiple times. Settings are applied
/// iteratively. Colors are limited to one of eight choices: red, blue,
/// green, cyan, magenta, yellow, white and black. Styles are limited to
/// nobold, bold, nointense, intense, nounderline or underline.
///
/// The format of the flag is '{type}:{attribute}:{value}'. '{type}' should
/// be one of path, line, column or match. '{attribute}' can be fg, bg
/// or style. '{value}' is either a color (for fg and bg) or a text
/// style. A special format, '{type}:none', will clear all color
/// settings for '{type}'.
///
/// For example, the following command will change the match color to
/// magenta and the background color for line numbers to yellow:
///
/// tree-sitter-grep --colors 'match:fg:magenta' --colors 'line:bg:yellow'
/// -q '(function_item) @f'
///
/// Extended colors can be used for '{value}' when the terminal supports
/// ANSI color sequences. These are specified as either 'x' (256-color)
/// or 'x,x,x' (24-bit truecolor) where x is a number between 0 and 255
/// inclusive. x may be given as a normal decimal number or a
/// hexadecimal number, which is prefixed by `0x`.
///
/// For example, the following command will change the match background
/// color to that represented by the rgb value (0,128,255):
///
/// tree-sitter-grep --colors 'match:bg:0,128,255'
///
/// or, equivalently,
///
/// tree-sitter-grep --colors 'match:bg:0x0,0x80,0xFF'
///
/// Note that the intense and nointense style flags will have no effect when
/// used alongside these extended color codes.
#[arg(long)]
pub colors: Vec<UserColorSpec>,

/// This flag controls when to use colors.
///
/// The default setting is 'auto', which means tree-sitter-grep will try to
/// guess when to use colors. For example, if tree-sitter-grep is printing
/// to a terminal, then it will use colors, but if it is redirected to a
/// file or a pipe, then it will suppress color output. tree-sitter-grep
/// will suppress color output in some other circumstances as well. For
/// example, if the TERM environment variable is not set or set to 'dumb',
/// then tree-sitter-grep will not use colors.
///
/// When the --vimgrep flag is given to tree-sitter-grep, then the default
/// value for the --color flag changes to 'never'.
#[arg(long, value_name = "WHEN")]
pub color: Option<ColorChoiceArg>,

/// This is a convenience alias for '--color always --heading
/// --line-number'.
///
/// This flag is useful when you still want pretty output even if you're
/// piping tree-sitter-grep to another program or file. For example:
/// 'tree-sitter-grep -p -q "(function_item) @c" | less -R'.
#[arg(short = 'p', long)]
pub pretty: bool,

/// This flag prints the file path above clusters of matches from each file
/// instead of printing the file path as a prefix for each matched line.
///
/// This is the default mode when printing to a terminal.
///
/// This overrides the --no-heading flag.
#[arg(long)]
pub heading: bool,

/// Don't group matches by each file.
///
/// If --no-heading is provided in addition to the -H/--with-filename flag,
/// then file paths will be printed as a prefix for every matched line.
/// This is the default mode when not printing to a terminal.
///
/// This overrides the --heading flag.
#[arg(long, overrides_with = "heading")]
pub no_heading: bool,

/// Display the file path for matches.
///
/// This is the default when more than one file is searched. If --heading is
/// enabled (the default when printing to a terminal), the file path will be
/// shown above clusters of matches from each file; otherwise, the file name
/// will be shown as a prefix for each matched line.
///
/// This flag overrides --no-filename.
#[arg(short = 'H', long)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you took this from ripgrep, but what's the "H" mean? "heading-ish"?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. At some point their algorithm may be "if we start running out of the "obvious short codes" then give the "less obvious" ones to less-used options"?

pub with_filename: bool,

/// Never print the file path with the matched lines.
///
/// This is the default when tree-sitter-grep is explicitly instructed to
/// search one file or stdin.
///
/// This flag overrides --with-filename.
#[arg(short = 'I', long, overrides_with = "with_filename")]
pub no_filename: bool,

/// Show line numbers (1-based).
///
/// This is enabled by default when searching in a terminal.
#[arg(short = 'n', long)]
pub line_number: bool,

/// Suppress line numbers.
///
/// This is enabled by default when not searching in a terminal.
#[arg(short = 'N', long, overrides_with = "line_number")]
pub no_line_number: bool,

/// Show column numbers (1-based).
///
/// This only shows the column numbers for the first match on each line.
/// This does not try to account for Unicode. One byte is equal to one
/// column. This implies --line-number.
///
/// This flag can be disabled with --no-column.
#[arg(long)]
pub column: bool,

#[arg(long, hide = true, overrides_with = "column")]
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hide/overrides_with seem (per a little reading) to be the most reasonable way to do these --no-* argument variants

pub no_column: bool,
}

impl Args {
fn use_paths(&self) -> Vec<PathBuf> {
pub fn use_paths(&self) -> Vec<PathBuf> {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting called in a couple different places now which seems like some unnecessary repeated allocations

So maybe I should make this effectively "memoized"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably write a macro which does the once-cell thing that you want to the function it annotates?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I certainly like that concept

Here though my instinct (now that we're starting to venture into crate-level API territory) is that we should only "cache"/"memoize" at the level of this particular Args instance (vs my understanding would be that a "purely-declarative" once-cell-ish thing would have to be using something static in "scope" under the hood?)

And so then interesting to think if that version is also something that could be expressed declaratively/via a macro

I don't know, what I'm picturing now is just an interior-mutable "private" field on Args that we call eg .get_or_init() on?

I guess you could avoid writing the .get_or_init() boilerplate (so keeping the function body as just the "actual value-calculation logic") by writing a macro eg #[memoize_via_once_cell_field(name_of_once_cell_field)]?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, I didn't really think about how it'd have to be per-instance.

That seems like a reasonable approach to me (the once cell field). I wonder if clap exposes some option to call a constructor for Args so that you could at startup calculate this stuff rather than having to do it lazily. (It's not like it's not going to be used, so having to do work to make it smartly lazy seems kind of silly.)

if self.paths.is_empty() {
vec![Path::new("./").to_owned()]
} else {
Expand All @@ -122,8 +261,26 @@ impl Args {
self.paths.is_empty()
}

fn line_number(&self) -> bool {
true
fn is_only_stdin(&self, paths: &[PathBuf]) -> bool {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are mimicking the ripgrep logic

paths == [Path::new("-")]
}

fn line_number(&self, paths: &[PathBuf]) -> bool {
// if self.output_kind() == OutputKind::Summary {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looked like logic we will want to "preserve" once we add eg JSON printing

// return false;
// }
if self.no_line_number {
return false;
}
// if self.output_kind() == OutputKind::JSON {
// return true;
// }

(grep_cli::is_tty_stdout() && !self.is_only_stdin(paths))
|| self.line_number
|| self.column
|| self.pretty
|| self.vimgrep
}

fn per_match(&self) -> bool {
Expand All @@ -135,7 +292,10 @@ impl Args {
}

fn column(&self) -> bool {
self.vimgrep
if self.no_column {
return false;
}
self.column || self.vimgrep
}

fn contexts(&self) -> (usize, usize) {
Expand All @@ -150,22 +310,27 @@ impl Args {
}
}

pub(crate) fn get_searcher(&self) -> Searcher {
pub(crate) fn get_searcher(&self, paths: &[PathBuf]) -> Searcher {
let (before_context, after_context) = self.contexts();
SearcherBuilder::new()
.line_number(self.line_number())
.line_number(self.line_number(paths))
.before_context(before_context)
.after_context(after_context)
.build()
}

pub(crate) fn get_printer(&self, buffer_writer: &BufferWriter) -> Printer {
pub(crate) fn get_printer(&self, paths: &[PathBuf], buffer_writer: &BufferWriter) -> Printer {
StandardBuilder::new()
.color_specs(self.color_specs())
.heading(self.heading())
.path(self.with_filename(paths))
.per_match(self.per_match())
.per_match_one_line(self.per_match_one_line())
.column(self.column())
.only_matching(self.only_matching)
.byte_offset(self.byte_offset)
.separator_context(self.context_separator())
.separator_search(None)
.build(buffer_writer.buffer())
}

Expand All @@ -190,4 +355,78 @@ impl Args {
) -> IterBridge<WalkParallelIterator> {
into_parallel_iterator(self.get_project_file_walker(), non_fatal_errors)
}

pub fn color_specs(&self) -> ColorSpecs {
let mut specs = default_color_specs();
for user_color_spec in &self.colors {
specs.push(user_color_spec.clone());
}
ColorSpecs::new(&specs)
}

pub fn buffer_writer(&self) -> BufferWriter {
let mut wtr = BufferWriter::stdout(self.color_choice());
wtr.separator(self.file_separator());
wtr
}

fn color_choice(&self) -> ColorChoice {
match self.color.unwrap_or(if self.vimgrep {
ColorChoiceArg::Never
} else {
ColorChoiceArg::Auto
}) {
ColorChoiceArg::Always => ColorChoice::Always,
ColorChoiceArg::Ansi => ColorChoice::AlwaysAnsi,
ColorChoiceArg::Auto => {
if grep_cli::is_tty_stdout() || self.pretty {
ColorChoice::Auto
} else {
ColorChoice::Never
}
}
ColorChoiceArg::Never => ColorChoice::Never,
}
}

fn heading(&self) -> bool {
if self.no_heading || self.vimgrep {
false
} else {
grep_cli::is_tty_stdout() || self.heading || self.pretty
}
}

fn file_separator(&self) -> Option<Vec<u8>> {
// if self.output_kind() != OutputKind::Standard {
// return Ok(None);
// }

let (ctx_before, ctx_after) = self.contexts();
if self.heading() {
Some(b"".to_vec())
} else if ctx_before > 0 || ctx_after > 0 {
self.context_separator()
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what is responsible for the fact that now in some of the existing test diffs you'll see that there are -- lines between files when passing a "context" option

(so we weren't matching ripgrep's behavior there basically, now we are)

} else {
None
}
}

fn context_separator(&self) -> Option<Vec<u8>> {
Some(b"--".to_vec())
}

fn with_filename(&self, paths: &[PathBuf]) -> bool {
if self.no_filename {
false
} else {
let path_stdin = Path::new("-");
self.with_filename
|| self.vimgrep
|| paths.len() > 1
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is causing it to not show the filename in the match output when you just pass a single file path (which is what ripgrep does)

|| paths
.get(0)
.map_or(false, |p| p != path_stdin && p.is_dir())
}
}
}
3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use std::{
use ignore::DirEntry;
use plugin::get_loaded_filter;
use rayon::prelude::*;
use termcolor::{BufferWriter, ColorChoice};
use thiserror::Error;
use tree_sitter::{Query, QueryError};

Expand Down Expand Up @@ -273,7 +272,7 @@ pub fn run(args: Args) -> Result<RunStatus, Error> {
get_loaded_filter(args.filter.as_deref(), args.filter_arg.as_deref())?.map(Arc::new);
let cached_queries: CachedQueries = Default::default();
let capture_index = CaptureIndex::default();
let buffer_writer = BufferWriter::stdout(ColorChoice::Never);
let buffer_writer = args.buffer_writer();
let matched = AtomicBool::new(false);
let searched = AtomicBool::new(false);
let non_fatal_errors: Arc<Mutex<Vec<NonFatalError>>> = Default::default();
Expand Down
15 changes: 7 additions & 8 deletions src/printer/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@ use std::{error, fmt, str::FromStr};

use termcolor::{Color, ColorSpec, ParseColorError};

#[allow(dead_code)]
pub fn default_color_specs() -> Vec<UserColorSpec> {
vec![
#[cfg(unix)]
"path:fg:magenta".parse().unwrap(),
#[cfg(windows)]
"path:fg:cyan".parse().unwrap(),
"line:fg:green".parse().unwrap(),
"match:fg:red".parse().unwrap(),
"match:style:bold".parse().unwrap(),
"path:fg:green".parse().unwrap(),
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that I had ripgrep's default colors overridden in a local config file

I can't with the default ripgrep colors they're disgusting

So this is "my" color scheme (don't remember where I got it from or if I made it up)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the highlights pretty gross when running in my terminal using my color scheme (I assume this is somehow using the colors I have specified there?):

Screenshot 2023-07-20 at 5 12 37 PM

But it's readable 🤷🏻

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm ya they probably chose theirs out of trying to be light-background- vs dark-background-terminal-agnostic

But I have a hard time stomaching using their default ones

Maybe (a) a more minimal version as the default

And (b) supporting a ripgrep-style config file?

We could even be sneaky and (ultimately in addition to looking for "our own" config file) look for someone's actual ripgrep config file (on the assumption that they would like similar "defaults" applied here vs ripgrep)?

Either way the fact that with n = 2 we have different desired highlighting tells me that making it (easily, vs passing --colors every time you use it) configurable is worth doing?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha yes, 100% of people want it to be configurable, I guess, and I definitely couldn't be bothered to pass --colors every time I used it.

"path:style:bold".parse().unwrap(),
"line:fg:yellow".parse().unwrap(),
"line:style:bold".parse().unwrap(),
"match:fg:black".parse().unwrap(),
"match:bg:yellow".parse().unwrap(),
"match:style:nobold".parse().unwrap(),
]
}

Expand Down
Loading