🎁 Terminal UI basic implementation #139

cardil · 2024-01-10T13:38:25Z

Changes

🎁 Terminal UI basic implementation

/kind enhancement

Refers to https://docs.google.com/document/d/15ZY2djwpALUXb-sRag3OksN42NR8zYTSPOggdGawzPY

knative-prow · 2024-01-10T13:38:28Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2024-01-31T12:04:48Z

Codecov Report

Attention: Patch coverage is 47.35883% with 289 lines in your changes are missing coverage. Please review.

Project coverage is 57.55%. Comparing base (ae3b852) to head (21c6ccc).

Files	Patch %	Lines
pkg/output/tui/progress.go	0.00%	166 Missing ⚠️
pkg/output/logging/context.go	81.18%	15 Missing and 4 partials ⚠️
pkg/output/tui/choose.go	0.00%	15 Missing ⚠️
pkg/context/wrapper.go	0.00%	14 Missing ⚠️
pkg/output/logging/zap.go	0.00%	12 Missing ⚠️
pkg/output/printer.go	0.00%	12 Missing ⚠️
pkg/config/dir/local.go	70.96%	6 Missing and 3 partials ⚠️
pkg/output/logging/logfile.go	80.00%	4 Missing and 3 partials ⚠️
pkg/output/testing.go	79.31%	4 Missing and 2 partials ⚠️
pkg/output/tui/print.go	0.00%	6 Missing ⚠️
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
- Coverage   58.26%   57.55%   -0.72%     
==========================================
  Files          94      113      +19     
  Lines        7833     8382     +549     
==========================================
+ Hits         4564     4824     +260     
- Misses       3027     3297     +270     
- Partials      242      261      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cardil · 2024-02-05T16:24:14Z

/cc @dsimansk
/cc @lkingland
/cc @matejvasek
/cc @matzew

Can I get a review, please?

cardil · 2024-02-07T10:52:27Z

Here's a usage in the ghet project: cardil/ghet@046f9b4

dsimansk

A few nits, around imports and used libs. One more significant wrt/ config logic and existing solution in other package.

In general the PR looks like a good start to start with Terminal UI. I have no strong opinions on widget interfaces. Let's polish those as suitable for implementation needs.

Out of curiosity, why is it called Terminal UI? Is it sort of de-facto standard for interactive CLIs? 🤔

pkg/config/dir/local.go

go.mod

pkg/config/dir/local.go

pkg/output/term/env.go

pkg/output/logging/context.go

go.mod

cardil · 2024-02-08T14:30:12Z

@dsimansk Out of curiosity, why is it called Terminal UI? Is it sort of de-facto standard for interactive CLIs? 🤔

Well, it's a UI of the terminal. Some people call it DUX (Developer UX), but that's a more broad term. I also heard CLI UX, but just once. Naming is hard...

cardil · 2024-02-08T14:36:16Z

pkg/output/tui/progress_test.go

+//go:build !race
+
+// TODO: there is a race condition in the progress code that needs to be fixed
+//       somehow


@dsimansk This is ugly.

There's a race condition, but I think it's in the bubblegum library itself, rather than my code.

Maybe we should investigate that, and at least open an issue on bubblegum project?!

I'm fine to keep it here for now. Given we either capture it in issue here at least or preferably on bubblegum lib with further details.

I'll try doing a reproducer versus the github.com/charmbracelet/bubbles/progress.

dsimansk · 2024-02-20T17:42:16Z

@cardil circling back to the topic, was there any update on the changes
?

cardil · 2024-02-21T15:35:15Z

@cardil circling back to the topic, was there any update on the changes ?

Not yet. In fact, today, I'm going back to this PR.

See: https://no-color.org/ and https://force-color.org/ for more information.

Conflicts fixed: - go.mod - go.sum - vendor/modules.txt

dsimansk · 2024-03-07T12:40:54Z

/approve
/lgtm

knative-prow · 2024-03-07T12:40:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, dsimansk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [dsimansk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dsimansk · 2024-03-07T12:41:42Z

As an initial impl that's suitable. We will refine it over time once put into use in other part of project.

@cardil thanks for pushing it forward!

dsimansk · 2024-03-07T12:42:01Z

/override codecov/project

knative-prow · 2024-03-07T12:42:04Z

@dsimansk: Overrode contexts on behalf of dsimansk: codecov/project

In response to this:

/override codecov/project

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow bot added kind/enhancement do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 10, 2024

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 10, 2024

knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 10, 2024

knative-prow bot requested review from itsmurugappan, skonto and vyasgun January 10, 2024 13:39

cardil force-pushed the feature/tui-codeimport branch 2 times, most recently from 576329d to 1caa1cc Compare January 31, 2024 12:00

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 31, 2024

cardil force-pushed the feature/tui-codeimport branch from 1caa1cc to 3846152 Compare January 31, 2024 12:10

cardil changed the title ~~[WIP] 🎁 Terminal UI basic implementation~~ 🎁 Terminal UI basic implementation Jan 31, 2024

cardil marked this pull request as ready for review January 31, 2024 18:00

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 31, 2024

knative-prow bot requested a review from ReToCode January 31, 2024 18:01

cardil force-pushed the feature/tui-codeimport branch from 3846152 to 40bbcdf Compare February 5, 2024 16:21

knative-prow bot requested review from dsimansk, lkingland, matejvasek and matzew February 5, 2024 16:24

cardil force-pushed the feature/tui-codeimport branch 6 times, most recently from fe80693 to bde83a5 Compare February 7, 2024 10:09

cardil force-pushed the feature/tui-codeimport branch from bde83a5 to 652a8eb Compare February 7, 2024 11:16

TUI code import from cardil/ghet

1e0edc0

cardil force-pushed the feature/tui-codeimport branch from 652a8eb to 1e0edc0 Compare February 7, 2024 12:26

dsimansk requested changes Feb 8, 2024

View reviewed changes

knative-prow bot assigned dsimansk Feb 8, 2024

cardil commented Feb 8, 2024

View reviewed changes

Documenting the environmental support for color

191bf3a

See: https://no-color.org/ and https://force-color.org/ for more information.

dsimansk mentioned this pull request Feb 28, 2024

Make package naming more consistent with skipping kn in the sub-dirs #152

Closed

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2024

cardil added 2 commits February 28, 2024 16:15

Remove github.com/kirsle/configdir dep

7dd518e

Merge remote-tracking branch 'upstream/main' into feature/tui-codeimport

21c6ccc

Conflicts fixed: - go.mod - go.sum - vendor/modules.txt

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2024

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2024

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2024

knative-prow bot merged commit f170971 into knative:main Mar 7, 2024
21 of 22 checks passed

cardil deleted the feature/tui-codeimport branch March 7, 2024 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎁 Terminal UI basic implementation #139

🎁 Terminal UI basic implementation #139

cardil commented Jan 10, 2024

knative-prow bot commented Jan 10, 2024

codecov bot commented Jan 31, 2024 •

edited

Loading

cardil commented Feb 5, 2024

cardil commented Feb 7, 2024 •

edited

Loading

dsimansk left a comment •

edited

Loading

cardil commented Feb 8, 2024

cardil Feb 8, 2024

dsimansk Feb 9, 2024

cardil Feb 28, 2024

dsimansk commented Feb 20, 2024

cardil commented Feb 21, 2024

dsimansk commented Mar 7, 2024

knative-prow bot commented Mar 7, 2024

dsimansk commented Mar 7, 2024

dsimansk commented Mar 7, 2024

knative-prow bot commented Mar 7, 2024

🎁 Terminal UI basic implementation #139

🎁 Terminal UI basic implementation #139

Conversation

cardil commented Jan 10, 2024

Changes

knative-prow bot commented Jan 10, 2024

codecov bot commented Jan 31, 2024 • edited Loading

Codecov Report

cardil commented Feb 5, 2024

cardil commented Feb 7, 2024 • edited Loading

dsimansk left a comment • edited Loading

Choose a reason for hiding this comment

cardil commented Feb 8, 2024

cardil Feb 8, 2024

Choose a reason for hiding this comment

dsimansk Feb 9, 2024

Choose a reason for hiding this comment

cardil Feb 28, 2024

Choose a reason for hiding this comment

dsimansk commented Feb 20, 2024

cardil commented Feb 21, 2024

dsimansk commented Mar 7, 2024

knative-prow bot commented Mar 7, 2024

dsimansk commented Mar 7, 2024

dsimansk commented Mar 7, 2024

knative-prow bot commented Mar 7, 2024

codecov bot commented Jan 31, 2024 •

edited

Loading

cardil commented Feb 7, 2024 •

edited

Loading

dsimansk left a comment •

edited

Loading