Skip to content

feat: --color#339

Draft
Copilot wants to merge 43 commits intomasterfrom
copilot/add-color-flag-and-mapping
Draft

feat: --color#339
Copilot wants to merge 43 commits intomasterfrom
copilot/add-color-flag-and-mapping

Conversation

Copy link
Contributor

Copilot AI commented Mar 6, 2026

Fixes #11.
Closes khai-slop-labs#1.

Adds --color [auto|always|never] support, coloring tree output using the LS_COLORS environment variable.

New components

  • ColorWhen enum (src/args/color.rs) — Auto (default, TTY-detect) / Always / Never
  • LsColors (src/ls_colors.rs) — wraps the lscolors crate; parses LS_COLORS into per-indicator ANSI prefix strings for Directory, Normal, Executable, Symlink
  • Color + Coloring (src/visualizer/coloring.rs) — Coloring holds a HashMap<PathBuf, Color> (full-path keys) and the LsColors prefixes; node_color(&Path, has_children) drives per-node color selection

Integration

  • Visualizer gains coloring: Option<&'a Coloring>; rows() wraps names in ANSI prefix/reset when set
  • build_coloring_map in sub.rs walks the DataTree, classifying leaves as Symlink > Directory > Executable (Unix) > Normal; directory coloring for internal nodes is resolved in the visualizer via has_children
  • App::run() evaluates TTY state + ColorWhen to construct and thread Coloring through to the visualizer

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
Copilot AI changed the title [WIP] Add --color flag for output coloring and path mapping Add --color=auto|always|never flag with LS_COLORS-aware file coloring Mar 6, 2026
@KSXGitHub KSXGitHub changed the title Add --color=auto|always|never flag with LS_COLORS-aware file coloring feat!: --color Mar 6, 2026
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Performance Regression Reports

commit: 5f4ca71

There are no regressions.

KSXGitHub

This comment was marked as resolved.

…ify rendering

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
KSXGitHub

This comment was marked as resolved.

…, update docs

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
KSXGitHub

This comment was marked as resolved.

… remove redundant attributes

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
KSXGitHub

This comment was marked as resolved.

…structure TreeHorizontalSlice

Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com>
KSXGitHub

This comment was marked as resolved.

Co-authored-by: Claude <noreply@anthropic.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

let coloring: Option<Coloring> = color.map(|ls_colors| {
let mut map = HashMap::new();
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

When pdu is run with multiple path arguments (e.g., pdu path1 path2), the code inserts a synthetic root node whose name is later changed from "" to "(total)" at line 147, before build_coloring_map is called at line 203. As a result, build_coloring_map constructs paths such as (total)/path1/file.txt and calls file_color on them. Since (total)/path1/file.txt does not exist on the filesystem, is_symlink(), is_dir(), and is_executable() all return false, and every file gets the default Color::Normal color — ignoring the actual file type. Symlinks and directories will be miscolored (shown as normal files) whenever --color is used with multiple path arguments. A fix would be to reconstruct each file's actual path from the child sub-trees before calling file_color, or to build the coloring map for each sub-tree individually using its original root path.

Suggested change
let mut map = HashMap::new();
let mut map = HashMap::new();
// NOTE: When `pdu` is invoked with multiple path arguments, a synthetic
// root node (renamed to "(total)") is created. Calling `build_coloring_map`
// on that synthetic root can lead to non-existent filesystem paths like
// "(total)/path1/file.txt" being passed to `file_color`, which in turn
// causes all filesystem-type checks (is_dir, is_symlink, etc.) to fail and
// results in incorrect coloring. The intended behavior is to build the
// coloring map per real subtree (each original argument) so that only
// actual filesystem paths are used for type detection.
//
// For now, we keep the single-root behavior here; special handling for
// synthetic aggregate roots should be implemented inside `build_coloring_map`
// (by skipping synthetic roots and traversing their children individually).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Colored output

3 participants