Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use chaining for AnsiGenericString::hyperlink #48

Merged
merged 1 commit into from
Jul 22, 2023

Conversation

sholderbach
Copy link
Member

The API for modifying a string to contain a hyperlink used in #47
behaves different than most of our Style-driven APIs that basically
all work through method chaining.

As an alternative I propose we change the signature to take the
AnsiGenericString by value and modify it in place. This feels more
expressive for quickly building out a styled string with a link.
One downside compared to a modifying setter API is that it takes a
more tokens to do conditional modification with an if expression.

cc @fdncred, @mhelsley

The API for modifying a string to contain a hyperlink used in nushell#47
behaves different than most of our `Style`-driven APIs that basically
all work through method chaining.

As an alternative I propose we change the signature to take the
`AnsiGenericString` by value and modify it in place. This feels more
expressive for quickly building out a styled string with a link.
One downside compared to a modifying setter API is that it takes a
more tokens to do conditional modification with an `if` expression.
@fdncred
Copy link

fdncred commented Jul 10, 2023

nice! I like this, seems intuitive when reading it to me. thanks stefan!

@fdncred
Copy link

fdncred commented Jul 10, 2023

I'm wondering if title should be changed too?

@sholderbach sholderbach merged commit b853460 into nushell:main Jul 22, 2023
2 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.

None yet

2 participants