Skip to content

getline: respect NO_COLOR when showing doc - #290

Open
tw4452852 wants to merge 1 commit into
janet-lang:masterfrom
tw4452852:patch-1
Open

getline: respect NO_COLOR when showing doc#290
tw4452852 wants to merge 1 commit into
janet-lang:masterfrom
tw4452852:patch-1

Conversation

@tw4452852

Copy link
Copy Markdown
Contributor

No description provided.

@sogaiu

sogaiu commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

I think there is also a dynamic variable *doc-color* which doc-format respects.

May be particular use cases can set *doc-color* via the NO_COLOR environment variable and the last parameter to the call here to doc-format can be removed?

@tw4452852

Copy link
Copy Markdown
Contributor Author

Ah, haven't noticed this exists.

However, this reminds me what if we handle NO_COLOR inside doc-format itself. Then all the callsites have the benefit automatically unless they force coloring with the parameter . What do you think?

@sogaiu

sogaiu commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

I don't have a clear idea of what would be good. Some thoughts follow:

  • In the context of boot.janet, things might be redundant because NO_COLOR handling happens already elsewhere via a call to apply-color. I guess the dynamic variable's value should take precedence?
  • The default behavior of default-doc-fetch is being changed. I don't know if anyone already relies on the previous behavior. I have had things break because of changes in color behavior elsewhere though and it's not something I wish on others.
  • doc-format's behavior becoming dependent directly on something external seems like a recipe for increasing unexpected behavior.

Hmmm...may be these are either non-issues or not really important? I guess it seems to me that it's not a clear win to change things, but the above are just initial thoughts.

It's not my call though in any case 😅

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