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.

Sign up for GitHub

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

Jump to bottom

feat(extensions/console): left align table entries #11295

Merged
merged 7 commits into from
Jul 7, 2021
Merged

feat(extensions/console): left align table entries #11295

merged 7 commits into from
Jul 7, 2021

Conversation

littledivy
Copy link
Member

Fixes #11294

Before:

┌───────────────┬───────────────────────────────────────────────────────────────────────────────────────────────┐
│     (idx)     │                                            Values                                             │
├───────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┤
│    alt-svc    │ 'h3-27=":443"; ma=86400, h3-28=":443"; ma=86400, h3-29=":443"; ma=86400, h3=":443"; ma=86400' │
│ cache-control │                                        "max-age=86400"                                        │
│    cf-ray     │                                    "66a7c2c3ab80f1ca-ATL"                                     │
│ content-type  │                                      "application/json"                                       │
│     date      │                                "Tue, 06 Jul 2021 09:21:04 GMT"                                │
│   expect-ct   │   'max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"'   │
│    server     │                                         "cloudflare"                                          │
│     vary      │                                       "Accept-Encoding"                                       │
└───────────────┴───────────────────────────────────────────────────────────────────────────────────────────────┘

After:

┌───────────────┬───────────────────────────────────────────────────────────────────────────────────────────────┐
│ (idx)         │ Values                                                                                        │
├───────────────┼───────────────────────────────────────────────────────────────────────────────────────────────┤
│ alt-svc       │ 'h3-27=":443"; ma=86400, h3-28=":443"; ma=86400, h3-29=":443"; ma=86400, h3=":443"; ma=86400' │
│ cache-control │ "max-age=86400"                                                                               │
│ cf-ray        │ "66a849f34cd20e64-BOM"                                                                        │
│ content-type  │ "application/json"                                                                            │
│ date          │ "Tue, 06 Jul 2021 10:53:23 GMT"                                                               │
│ expect-ct     │ 'max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"'     │
│ server        │ "cloudflare"                                                                                  │
│ vary          │ "Accept-Encoding"                                                                             │
└───────────────┴───────────────────────────────────────────────────────────────────────────────────────────────┘

(example taken from linked issue)

@littledivy littledivy marked this pull request as ready for review July 6, 2021 11:41
SimonRask
SimonRask suggested changes Jul 6, 2021
View reviewed changes
extensions/console/02_console.js Outdated Show resolved Hide resolved
jsejcksn
jsejcksn suggested changes Jul 7, 2021
View reviewed changes
extensions/console/02_console.js Outdated Show resolved Hide resolved
extensions/console/02_console.js Outdated Show resolved Hide resolved
@bartlomieju bartlomieju mentioned this pull request Jul 7, 2021
Closed
littledivy and others added 4 commits July 7, 2021 10:03
@littledivy @jsejcksn
Update extensions/console/02_console.js
affc080 
Co-authored-by: Jesse Jackson <jsejcksn@users.noreply.github.com>
@littledivy @jsejcksn
Update extensions/console/02_console.js
9ed476d 
Co-authored-by: Jesse Jackson <jsejcksn@users.noreply.github.com>
@littledivy
fix lint
dfceeed
@littledivy @SimonRask
Add SimonRask as co-author
06af9d2 
Co-authored-by: Simon Rask <SimonRask@users.noreply.github.com>
@littledivy
Copy link
Member Author

Thanks for the review @SimonRask and @jsejcksn. I've added both as co-authors.

ry
ry approved these changes Jul 7, 2021
View reviewed changes
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Thank you!

@ry ry merged commit a8dcf9e into denoland:main Jul 7, 2021
@ry
Copy link
Member

ry commented Jul 7, 2021

Oh shit - I didn't maintain the co-author attributes for @SimonRask and @jsejcksn. I'm very sorry! Thank you for your contribution!

@littledivy littledivy deleted the console/left-align-table branch July 7, 2021 15:57
@jsejcksn
Copy link
Contributor

jsejcksn commented Jul 7, 2021

Oh shit - I didn't maintain the co-author attributes for @SimonRask and @jsejcksn. I'm very sorry! Thank you for your contribution!

@ry No problem — I'm just glad it was merged.

It makes me wonder though:

  • What is the process for this? and
  • Can there be a check in place to help with that?

@kitsonk
Copy link
Contributor

kitsonk commented Jul 7, 2021

Not really. When we merge something GitHub helpfully generates the co-author headers, but it is up to the commiter to remember to not delete them. While we could use something outside of GitHub to do the squash merge commits it likely would only cause other problems and make the workflow more complex.

@jsejcksn
Copy link
Contributor

jsejcksn commented Jul 7, 2021

@kitsonk Thanks for responding. While the process wasn't exactly explained, it sounds like this is being done manually by the committer in a browser using the GitHub web UI.

we could use something outside of GitHub to do the squash merge commits

There are many configurable GitHub bots available to help with this task. While it would necessarily add a bit of complexity, it would help by making the commit process more deterministic and implementing a desired procedure (e.g. enforcing conventional commit syntax, trimming out all of the granular commit history messages while maintaining co-authors, etc.).

nicolasstucki added a commit to nicolasstucki/deno that referenced this pull request Aug 17, 2021
@nicolasstucki
feat(extensions/console): right align numerical columns in table
4a4fab9 
Fixes denoland#11295.

The values of a column are left aligned if and only if the column contains at least one non-numerical value.

```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│ 0     │ "apples"  │
│ 1     │ "oranges" │
│ 2     │ "bananas" │
└───────┴───────────┘
```

```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│     0 │ "apples"  │
│     1 │ "oranges" │
│     2 │ "bananas" │
└───────┴───────────┘
```
nicolasstucki added a commit to nicolasstucki/deno that referenced this pull request Aug 17, 2021
@nicolasstucki
feat(extensions/console): right align numeric columns in table
79b510f 
Fixes denoland#11295.

The values of a column are left aligned if and only if the column contains at least one non-numerical value.

```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│ 0     │ "apples"  │
│ 1     │ "oranges" │
│ 2     │ "bananas" │
└───────┴───────────┘
```

```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│     0 │ "apples"  │
│     1 │ "oranges" │
│     2 │ "bananas" │
└───────┴───────────┘
```
nicolasstucki added a commit to nicolasstucki/deno that referenced this pull request Aug 17, 2021
@nicolasstucki
feat(extensions/console): right align numeric columns in table
b91eec5 
Fixes denoland#11295.

The values of a column are left aligned if and only if the column contains at least one non-numerical value.

Before
```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│ 0     │ "apples"  │
│ 1     │ "oranges" │
│ 2     │ "bananas" │
└───────┴───────────┘
```

After
```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│     0 │ "apples"  │
│     1 │ "oranges" │
│     2 │ "bananas" │
└───────┴───────────┘
```
@nicolasstucki nicolasstucki mentioned this pull request Aug 17, 2021
Merged
nicolasstucki added a commit to nicolasstucki/deno that referenced this pull request Aug 18, 2021
@nicolasstucki
feat(extensions/console): right align numeric columns in table
8f3c8c3 
Fixes denoland#11295.

The values of a column are left aligned if and only if the column contains at least one non-numerical value.

Before
```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│ 0     │ "apples"  │
│ 1     │ "oranges" │
│ 2     │ "bananas" │
└───────┴───────────┘
```

After
```
┌───────┬───────────┐
│ (idx) │ Values    │
├───────┼───────────┤
│     0 │ "apples"  │
│     1 │ "oranges" │
│     2 │ "bananas" │
└───────┴───────────┘
```
@waldyrious waldyrious mentioned this pull request Jan 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsejcksn jsejcksn jsejcksn requested changes

@SimonRask SimonRask SimonRask requested changes

@ry ry ry approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improve console.table alignment
5 participants
@littledivy @ry @jsejcksn @kitsonk @SimonRask