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

Core Summary should be more flexible #1544

Open
jrkarnes opened this issue Oct 2, 2023 · 4 comments
Open

Core Summary should be more flexible #1544

jrkarnes opened this issue Oct 2, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@jrkarnes
Copy link

jrkarnes commented Oct 2, 2023

If your issue is relevant to this repository, please include the information below:

Describe the enhancement
The summary tools in @actions/core should be more flexible in their application. There are formatting things that should be possible which are currently not supported by the interface and implementation. Some choice examples are enumerated here:

  1. You cannot produce markdown links with summary.addLink() in a table being produced by summary.addTable()
  2. There is no table builder and you cannot produce a table through the summary tool row-by-row, everything has to be provided to the addTable method at once.
  3. You cannot wrap elements in the <detail> block using addDetail as it only accepts a string.

These are just a few of the gripes I've run into while using the summary tools in @actions/core, I'm sure there are plenty others.

I would suggest that the approach to how the summary is handled be altered such that "a summary" be composed of an array of composable elements. When it comes time to write the summary, the elements resolve themselves to their string representations and then be written.

Code Snippet
Example follows:

import * as core from '@actions/core'
core.summary
  .addHeading('Top Level Heading')
  .startDetail('Details Summary Text')
    .addTable(
      new core.summary.table
        .withHeaderRow(['column header', 'column header', 'column header'])
        .withRow(['a', 'single', 'row'])
        .withRows([['or', 'an', 'array'], ['of', 'rows', 'together']])
        .withRow(['or', 'even', core.summary.addLink('a link!', 'https://www.zombo.com')])
      .finalize()
    )
  .write()

Additional information
If a fully developed composable element library isn't feasible, can we at least expose a method to export the string representation of what core.summary.addX() methods would write so that we can patch together a desired output?

@jrkarnes jrkarnes added the enhancement New feature or request label Oct 2, 2023
@jamesrenaud
Copy link
Contributor

You wouldn't know it because the documentation for summary is for some reason completely missing in the docs, but you can use the .addRaw() method to give the summary your own GitHub flavoured markdown.

const myMarkdown = `## My Header

---
Some stuff here :green_circle: With a [link](https://github.com)

### Maybe Add A table

| Header1 | Header2 | Header3 |
|--- |--- | --- |
| value1 | value2 | value |
`

await core.summary.addRaw(myMarkdown).write()

@jrkarnes
Copy link
Author

jrkarnes commented Oct 2, 2023

@jamesrenaud I appreciate the suggestion, but then you're having to build your own markdown dynamically in some external hamster-wheel rather than leveraging the functionality of the core toolkit. This likely means that you're pulling in some kind of markdown parser specific to github flavored markdown and using it to just generate a string.

Now while there's nothing inherently wrong with that, it does feel a little bit kludgy to require an external tool (even if that tool is your brain) to generate a raw markdown string if you want the formatting to be precise... Specifically the weird inability to put a link in a markdown document with the addTable method (which only takes string[] to resolve to SummaryTableRow[]) since a markdown link generated by addLink should just be string.

Using your example:

const mdString = []
mdString.push("## My Header")
mdString.push('### Maybe Add A Table')
mdString.push('| Header | Header | Header|')
// Here is where we get into some semantic questions
// Do you do something like:
mdString.push(MarkdownTableOptions.tableHeaderSeparator({cols: 3, alignment: left}))
// Or do you have to:
mdString.push('|--- |--- | --- |')
// It feels really bad to have to define the raw text for formatting markdown in source.
for (const row in/of someIterableObject) {
  // Here lies logic for building a row of a table
  // but before you decide how to assemble the damn thing
  // you have to know how mdString is being handled because...
}
// When this happens everything explodes badly for you:
mdString.join("/n")

How did you decide to build your rows? Did you mdString.push('| ') and then in the for loop do mdString.push('<dynamic_stuff> |')? If you did, now your formatting breaks.

Did you decide to do mdString.push(<entire_row_in_one_string_because_that's_always_going_to_look_clean_in_code_and_never_ever_look_bad_if_you_have_long_text_entries>)?

A different perspective on that whole mess is: "Why should I have to know how I'm going to be joining my elements together at the end of this process to determine what happens in the middle? Shouldn't encapsulation inherently solve this problem?"

It wouldn't be so terrible if these summary.addX methods were just wrappers to channel string content into a buffer and we had access to the underlying strata that produces the content... but we don't.

Consider the alternative (using the example above):

import * as core from '@actions/core'
core.summary.addHeader('My Header', 2)
core.summary.addHeader('Maybe Add A Table', 3)
core.summary.startTable()
core.summary.addHeaderRow(['Header1', 'Header2', 'Header3'])
core.summary.configureAlignmentOptions(['left', 'left', 'center'])
for (const someIterable of/in someCollection) {
  const builtRow = []
  // Add elements as required
  core.summary.addRow(builtRow)
}
core.summary.finishTable()
core.summary.write()

The startTable() method now acts as kind of a "what comes next" validator toggle. If you're currently in table mode, you're putting things in a table until you stop being in a table. Google took this same approach with GSON and custom object encoding and I personally think its the right way to go. You can extend the pattern with details and all the other fancy <> script that github flavored markdown supports. For example:

core.summary.startDetailsBlock()
core.summary.addSummary('Hey look, a dropdown!')
core.summary.addX()
core.summary.finishDetailsBlock()

Now you're building a details section and you can put arbitrary stuff in it without breaking out of the core.summary() supported methods.

Having said all that, I thoroughly appreciate you helping to unblock my work. At this point the feature request is more for how I would have expected the paradigm to be. If the team decides to not pick it up, I won't like it but I will understand the decision.

@MikeMcC399
Copy link

MikeMcC399 commented Oct 13, 2023

@jamesrenaud

You wouldn't know it because the documentation for summary is for some reason completely missing in the docs,

Agreed!

https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary doesn't describe how to use @actions/core.summary.

There is an example in a blog post https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/ however the only way to discover the full functionality is to read the code! https://github.com/actions/toolkit/blob/main/packages/core/src/summary.ts unless I've missed some other documentation hidden away somewhere!

@jamesrenaud
Copy link
Contributor

@jrkarnes At the end of the day the core.summary methods are meant to be simple helpers to the job summary - each one just spits out some fairly straightforward HTML with some optional attributes to the tags.

As with any helper methods, once your requirements start to get complex, or you want to control additional HTML attributes, the addRaw() method exposes the job summary to whatever you want to do with it. They've done a good job here exposing that so you can add one big string of your finished content or line by line through your logic.

Getting really opinionated in these helpers just makes the code difficult to maintain, and you're still never going to cover everyone's use case.

@MikeMcC399 I've got a PR inbound with some (hopefully) helpful documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants