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

Introduce MarkupInterpolated and MarkupLineInterpolated extensions #564

Closed
wants to merge 2 commits into from

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Sep 29, 2021

These new methods enable easily writing markup with a nice and intuitive syntax without having to worry about escaping the markup.

string input = args[0];
string output = Process(input);
AnsiConsole.MarkupLineInterpolated($"[blue]{input}[/] -> [green]{output}[/]");

The Interpolated suffix was inspired by the Entity Framework Core FromSqlInterpolated method.

These new methods enable easily writing markup with a nice and intuitive syntax without having to worry about escaping the markup.

```csharp
string input = args[0];
string output = Process(input);
AnsiConsole.MarkupLineInterpolated($"[blue]{input}[/] -> [green]{output}[/]");

```

The `Interpolated` suffix was inspired by the Entity Framework Core [FromSqlInterpolated][1] method.

[1]: https://docs.microsoft.com/en-us/ef/core/querying/raw-sql#passing-parameters
@patriksvensson
Copy link
Contributor

Is there a related issue for this PR?

@phil-scott-78
Copy link
Contributor

@0xced, I've been working on something similar that I wanted to get done with a RFC. It has the same idea you have but takes it a bit further by also adding the ability to format the parameters with styling.

Since you and I seem to be looking in the same direction, think you could be the first to let me know what you think about how this works?

My POC is here - https://github.com/phil-scott-78/Spectre.Console.StringInterpolation

@phil-scott-78
Copy link
Contributor

Been thinking about this some more. Your PR is far more polished than what I put together so I think it should be the base. I can extend it without any breaking changes so it's a better starting point.

My hangup is the name. I like the idea enough that I'm strongly tempted to make it the recommended way to call it every time. And if so it kinda feels like including Interpolated in the name, while accurate, might be a bit too verbose.

Thoughts on changing it to MarkupFormat and MarkupFormatLine? If we went this route I might also provide overrides to match string.Format signatures too for people like F# users that might not want to use the interpolated strings but still want the autoformatting.

@0xced
Copy link
Contributor Author

0xced commented Nov 10, 2021

And if so it kinda feels like including Interpolated in the name, while accurate, might be a bit too verbose.

I totally agree with you. I went with the Interpolated suffix because this is how Entity Framework Core does it with FromSqlInterpolated and I don't know of any other example of .NET code taking advantage of the FormattableString type, but I haven't really looked.

Thoughts on changing it to MarkupFormat and MarkupFormatLine?

I like them better but I'm afraid they sound too much like methods using composite formatting which is exactly what they are not!

If we went this route I might also provide overrides to match string.Format signatures too for people like F# users that might not want to use the interpolated strings but still want the autoformatting.

I'm not familiar with F# at all and I don't see how your proposal would look like. Care to elaborate?

Anyway, I'm really open to something else, I don't particularly like Interpolated either.


By the way, can someone with write access to the repository (probably Patrik or yourself) turn this pull request into a draft?

@nils-a nils-a marked this pull request as draft November 14, 2021 22:02
@nils-a
Copy link
Contributor

nils-a commented Nov 14, 2021

can someone with write access to the repository (probably Patrik or yourself) turn this pull request into a draft?

Done.

@phil-scott-78 phil-scott-78 marked this pull request as ready for review February 27, 2022 14:16
Copy link
Contributor

@phil-scott-78 phil-scott-78 left a comment

Choose a reason for hiding this comment

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

We are going through the backlog and this is one item that we (well, I) let sit too long.

@0xced if you get a chance can you review the conflicts? Or alternatively I can build off of what you have and get it merged

@0xced
Copy link
Contributor Author

0xced commented Feb 28, 2022

I'm away from computer for this week, so please go ahead as you see fit, Phil.

@phil-scott-78
Copy link
Contributor

I'm a bit concerned I broke github by cheating and using their UI to resolve the merge conflict....

@phil-scott-78
Copy link
Contributor

I've built this out a bit more with #761 based on your original commit after I made a mess of this one. Added some documentation and an additional helper method. I'm going to close this PR in favor of the other, but I hope you know that doesn't mean the contribution wasn't appreciated :-)

@0xced 0xced deleted the MarkupInterpolated branch September 18, 2022 13:58
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

4 participants