Skip to content

Conversation

vinxcls
Copy link
Contributor

@vinxcls vinxcls commented Jul 2, 2025

Issue #54

This patch enhances bfs by processing the LSCOLORS environment variable. It fills in any missing values with defaults ones.
If LS_COLORS is already set and configured, bfs will continue to use those settings, ensuring backward compatibility.

Copy link
Owner

@tavianator tavianator left a comment

Choose a reason for hiding this comment

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

Thanks for doing this for me! I have some comments, but the code is very close to mergeable. Nice work!

@vinxcls vinxcls marked this pull request as draft July 3, 2025 07:43
@vinxcls vinxcls force-pushed the freebsd-coloring branch from 3ad0775 to 5e6df3c Compare July 3, 2025 10:34
@vinxcls vinxcls marked this pull request as ready for review July 3, 2025 10:35
@vinxcls vinxcls requested a review from tavianator July 3, 2025 10:35
Copy link
Owner

@tavianator tavianator left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments! Just a few even more minor nits, mostly style things, and this should be good to go.

If LSCOLORS is undefined or partially defined (missing some of the eleven expected
value pairs), the patch will automatically fill in those missing values with default
colors. However, if the LS_COLORS environment variable is already defined, the
application prefer the LS_COLORS approach, maintaining current working flow of the
program.
@vinxcls vinxcls force-pushed the freebsd-coloring branch from feb01c5 to b4a8afc Compare July 3, 2025 12:32
@vinxcls vinxcls requested a review from tavianator July 3, 2025 12:33
@vinxcls
Copy link
Contributor Author

vinxcls commented Jul 3, 2025

Thanks a lot for approving the PR... even more for your helpful guidance!

@tavianator tavianator merged commit f1fa639 into tavianator:main Jul 3, 2025
17 of 18 checks passed
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.

2 participants