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

feat/write list irenderable #1438

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jos3s
Copy link

@jos3s jos3s commented Jan 30, 2024

  • new overload of the write method receiving IEnumerable or IRenderable[] params
  • implementing a new overload in the classes that implement IAnsiConsole
  • implementing a new overload in the TestConsole
  • new unit tests for Write, TextPath, BarChart and Table

fixes #1437

  • I have read the Contribution Guidelines
  • I have commented on the issue above and discussed the intended changes
  • A maintainer has signed off on the changes and the issue was assigned to me
  • All newly added code is adequately covered by tests
  • All existing tests are still running without errors
  • The documentation was modified to reflect the changes OR no documentation changes are required.

Changes

I created an overload in the Write method of the IAnsiConsole interface and implemented it in all the classes that implement this interface. The purpose of this overload is to receive a list of IRenderable objects and render them together without having to call AnsiConsole.Write for each IRenderable object.


Please upvote 👍 this pull request if you are interested in it.

@patriksvensson
Copy link
Contributor

Wouldn't adding an extension method for IAnsiConsole that accomplishes the same thing suffice?

public static void Write(this IAnsiConsole console, params IRenderable[] renderables)
{
    foreach(var renderable in renderables) 
    {
        console.Write(renderable);
    }
}

@jos3s
Copy link
Author

jos3s commented Jan 31, 2024

Yes, it could be. When analyzing the code, I noticed that AnsiConsole's extension methods always receive pure values ("string", double, int and others), so I thought that in this case, since it will receive an IRenderable, it would be a class method rather than an extension.

I will remove these changes and just add an extension method that receives the IRenderable[] params

@jos3s
Copy link
Author

jos3s commented Jan 31, 2024

@patriksvensson

I don't know why the build is breaking, I looked for the file displayed in the log and it doesn't exist

/home/runner/work/spectre.console/spectre.console/test/Spectre.Console.Tests/Unit/Widgets/ProgressBarTests.cs(3,2): error CS0619: 'UsesVerifyAttribute' is obsolete: 'No longer required. Usages of this attribute can be removed.'

*Resolved

@jos3s jos3s force-pushed the feat/write-list-irenderable branch from 819c003 to 1bb1e60 Compare January 31, 2024 21:17
@jos3s jos3s requested a review from patriksvensson January 31, 2024 21:22
@FrankRay78
Copy link
Contributor

Looks like a nice clean implementation @jos3s at first glance, I'll prioritise reviewing this in the coming week.

@FrankRay78 FrankRay78 self-assigned this Feb 1, 2024
@FrankRay78 FrankRay78 self-requested a review February 1, 2024 22:31
@patriksvensson
Copy link
Contributor

@FrankRay78 I've already started the review

@FrankRay78 FrankRay78 removed their assignment Feb 1, 2024
@FrankRay78 FrankRay78 removed their request for review April 15, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

AnsiConsole.Write receives an IEnumerable from IRenderable
3 participants