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

Add a new tree-view form for the progress view. #1260

Merged
merged 31 commits into from
Apr 24, 2018
Merged

Conversation

CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Apr 23, 2018

Updated the pulumi console to print out the tree-structure of a program's resources when in a tty session.

The tree-structure updates live, but doesn't come across as distracting or hard to understand.

Looks like this:

image

@@ -82,6 +83,10 @@ type ProgressDisplay struct {
// Any system events we've received. They will be printed at the bottom of all the status rows
systemEventPayloads []engine.StdoutEventPayload

// Used to record the order that rows are created in. That way, when we present in a tree, we
// can keep things ordered so they will not jump around.
displayTime int
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I think the Time suffix here is confusing. I would have expected this was going to a timestamp or something, but it looks like it's just a counter or row index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah.. Do you have a better name? It's more a way to just know the display ordering. Perhaps displayOrderIndex?

Copy link
Contributor

Choose a reason for hiding this comment

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

displayOrderIndex sounds great!

@@ -81,15 +82,17 @@ func TrimColorizedString(v string, maxLength int) string {
contract.Assertf(!tagRegexp.MatchString(textOrTag), "Got a tag when we did not expect it")

text := textOrTag
if currentLength+len(text) > maxLength {
textLen := utf8.RuneCountInString(text)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect the interaction between counting runes but trimming on byte boundaries is going to lead to wackiness in some cases. Is there a reason we moved to utf8.RuneCountInString?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. certain characters like the tree-characters are 3 len long, but only one rune. You are correct though, the trimming based on length is totally bogus, i missed that, thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit 42d7174 into master Apr 24, 2018
@CyrusNajmabadi CyrusNajmabadi deleted the treeView branch April 24, 2018 18:13
@lindydonna
Copy link

This looks awesome! @CyrusNajmabadi Do you mind adding a line of descriptive text to the PR, to make changelog generation easier?

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