Clean up `state search` UI #3099

MDrakos · 2024-02-11T22:28:21Z

DX-2400 Clean up the `state search` UI

This uses the viewport bubble to handle rendering and scrolling. Example here for reference.

internal/runners/packages/search.go

Naatan · 2024-02-12T20:58:24Z

internal/runners/packages/search.go

+	table, err := createSearchTable(v.width, v.height, processedPackages, vulns)
+	if err != nil {
+		return errs.Wrap(err, "Could not create search table")
+	}
+	v.content = table.Content()
+	v.packageNames = table.packageNames


It feels like the table should itself be the view.

I've updated this a bit so that the view is more encapsulated. We are still using this type to process the search results but any view processing for the viewport is now done elsewhere.

Naatan · 2024-02-12T21:00:26Z

internal/runners/packages/search.go

+	if s.out.Type().IsStructured() {
+		s.out.Print(table)


Handling structured output through the table feels overloaded. It seems the table is both responsible for handling view logic and for handling data parsing. I'd try to calculate the structured data first and foremost, and if that's all the user requested we never even touch any bubbletea or view logic.

I've moved the table view calculation to the actual view type. We are still using this table type to calculate the structured search results and the view does further processing for the plain output. Otherwise we dump the table from the search results.

internal/runners/packages/search.go

Naatan · 2024-02-12T23:35:31Z

internal/runners/packages/searchView.go

+}
+
+func NewView() (*view, error) {
+	width, height, err := term.GetSize(int(os.Stdout.Fd()))


Don't use os directly, instead use output.Config().OutWriter.

This is expecting a file descriptor. The OutWriter is an io.Writer interface.

Can we add the Fd() method on Outwriter? I'm not confident that we'll end up using this long term but having everything agree on a same source of truth will make refactoring this later easier.

Added a method to the output Config type.

internal/runners/packages/searchView.go

Also update index

Co-authored-by: Nathan Rijksen <nathanr@activestate.com>

Naatan

I can't select text in my terminal when viewing search results. There's a weird symbol on my cursor that I think indicates it has some sort of lock. Possibly this is something the viewport imposes, but we might be able to configure it?

internal/runners/packages/searchView.go

internal/runners/packages/search.go

Naatan · 2024-02-14T21:16:19Z

internal/runners/packages/searchView.go

+		cmds []tea.Cmd
+	)
+
+	switch msg := msg.(type) {


It looks like scrolling is already handled through the viewport lib:

https://github.com/charmbracelet/bubbles/blob/master/viewport/viewport.go#L287

Is there a reason we can't make use of that?

Took some time to try and use this. The Update method does not implement the correct interface so it's not a simple replacement. We would likely have to do some wrapping. I didn't realize the viewport had these methods as I was following the example that I linked in my initial comment.

Can't you just call the viewport's Update() method from your Update() method? ie. handle whatever you need to handle, then defer the rest to the viewport package.

Took another look at this and no, it's not that simple. Again, the Update() function return a struct that doesn't satisfy the tea.Model interface. This means that we can't just return the values that the Update() function returns.

I didn't spend a lot of time trying to get this working but I managed to by replacing the underlying viewport. Unfortunately, this means we lose the header and footer spacing that we want to setup so it looks a bit off.

Looking at bit more at the code you linked it's not immediately evident what benefit we get from using this Update function. It would be handling the scrolling keys for us but that's about it.

If there is some other benefit or if this is something that we need before this can be merged I can try to get it working but it will take some time. If you have an idea on how this might work that I'm not seeing feel free to take a stab at it or just let me know.

internal/runners/packages/searchView.go

internal/runners/packages/styles.go

pkg/platform/model/vulnerabilities.go

Co-authored-by: Nathan Rijksen <nathanr@activestate.com>

MDrakos · 2024-02-14T23:37:44Z

I can't select text in my terminal when viewing search results. There's a weird symbol on my cursor that I think indicates it has some sort of lock. Possibly this is something the viewport imposes, but we might be able to configure it?

Unfortunately, it doesn't appear that this is currently supported: charmbracelet/bubbletea#162

mitchell-as · 2024-02-15T14:58:46Z

Try holding down Shift and click-dragging with the mouse. At least in Linux this should work with terminals whose current applications are handling mouse events.

Naatan · 2024-02-15T18:08:18Z

I can't select text in my terminal when viewing search results. There's a weird symbol on my cursor that I think indicates it has some sort of lock. Possibly this is something the viewport imposes, but we might be able to configure it?

Unfortunately, it doesn't appear that this is currently supported: charmbracelet/bubbletea#162

Bah.. ok let's revert mouse scrolling then. It's more important that you can select text than that you can scroll with your mouse. And git log apparently has the same limitation.

@mitchell-as shift+click didn't do anything for me in iTerm :\

Naatan · 2024-02-15T18:12:14Z

internal/runners/packages/searchresult.go

+		result.Name = ptr.From(pkg.Ingredient.Name, "")
+		result.Description = ptr.From(pkg.Ingredient.Description, "")
+		result.Website = pkg.Ingredient.Website.String()
+		result.License = ptr.From(pkg.LatestVersion.LicenseExpression, "")


Author information is missing.

I should have noted this somewhere but we discussed during stand that retrieving the author information requires us to make a separate API call for each package in the search results. We deemed that this would slow down the command too much so opted to leave the authors out.

Thanks, forgot about that. I filed DX-2581 and a blocker with XT to hopefully get this resolved.

internal/runners/packages/searchView.go

Naatan

Nearly there, two remaining issues:

Vulnerabilities is not colored correctly, and has inconsistent indentation:

Authors is still missing.

MDrakos · 2024-02-15T22:23:14Z

Missed the vulnerabilities key on the last round of changes. Should be updated now as well as the vulnerability colours.

As for the authors see this comment: #3099 (comment)

Naatan

Great work!

MDrakos added 3 commits February 6, 2024 10:40

Initial WIP

eae0e87

Basic output working

48d22f8

Use 3rd party libraries for formatting

00b7fd0

MDrakos added the Test: package label Feb 11, 2024

MDrakos changed the base branch from master to version/0-44-0-RC1 February 11, 2024 22:30

MDrakos added 10 commits February 11, 2024 14:51

Fix footer and integration tests

fcaec1c

More test fixes

d942b7b

Add structured output

8969871

Capitalize expect

07bb0ed

Use localization

8f1b597

Revert .gitignore changes

a75aa7a

Remove hard coded padding from key locales

6e70b2c

Renaming

7eb5dea

More cleanup

3f8aa9e

Reorganize color styles because OCD

75837bb

MDrakos requested a review from Naatan February 12, 2024 18:23

Naatan requested changes Feb 12, 2024

View reviewed changes

Move latest version processing to the inventory model package

af39d4d

Base automatically changed from version/0-44-0-RC1 to beta February 13, 2024 17:56

MDrakos added 6 commits February 13, 2024 13:45

Separate view from results processing

1b20277

Move vulnerabilities counting to model package

dc53779

User ptr.from

84a941b

Implement mouse scrolling

fa37c0b

Also update index

Fix build error

25dde97

Add comments

b50bde6

MDrakos changed the base branch from beta to version/0-44-0-RC1 February 14, 2024 00:44

MDrakos and others added 4 commits February 13, 2024 16:47

Localize key

4f20113

Update internal/runners/packages/searchView.go

521f454

Co-authored-by: Nathan Rijksen <nathanr@activestate.com>

Make index an actual index

e1cec82

Support page up and down

5a5f4d6

MDrakos and others added 6 commits February 14, 2024 09:44

Update internal/runners/packages/searchView.go

caade80

Co-authored-by: Nathan Rijksen <nathanr@activestate.com>

Get output writer file descriptor

b96eb8b

Rename files

5d68824

Move constants and variables

56c0e85

Remove view logic from search results creation

ac4dc90

Use constants, add docstring

8e41d23

MDrakos requested a review from Naatan February 14, 2024 19:22

Naatan requested changes Feb 14, 2024

View reviewed changes

MDrakos and others added 6 commits February 14, 2024 13:41

Update internal/runners/packages/searchView.go

2ac5b93

Co-authored-by: Nathan Rijksen <nathanr@activestate.com>

Remove errMsg

211b98e

Move color styles

773982b

Use constant keys for vulnerabilities

d93e77f

Fix omitEmpty option

90a5761

Remove index

dd4a3f0

MDrakos requested a review from Naatan February 15, 2024 00:20

Naatan requested changes Feb 15, 2024

View reviewed changes

MDrakos added 2 commits February 15, 2024 12:52

Remove mouse support

bb9fa3c

Use light grey styling for keys

026cdab

MDrakos requested a review from Naatan February 15, 2024 21:55

Naatan requested changes Feb 15, 2024

View reviewed changes

Correct vulnerabilities key and update vuln colors

1b09490

MDrakos requested a review from Naatan February 15, 2024 22:30

Naatan approved these changes Feb 15, 2024

View reviewed changes

MDrakos merged commit 65b39c6 into version/0-44-0-RC1 Feb 15, 2024
6 of 7 checks passed

MDrakos deleted the DX-2400 branch February 15, 2024 22:47

as-builds added a commit that referenced this pull request Feb 15, 2024

Merge branch version/0-44-0-RC1 to adopt changes from PR #3099

64abf2b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up `state search` UI #3099

Clean up `state search` UI #3099

MDrakos commented Feb 11, 2024 •

edited by github-actions bot

Loading

Naatan Feb 12, 2024

MDrakos Feb 14, 2024

Naatan Feb 12, 2024

MDrakos Feb 14, 2024

Naatan Feb 12, 2024

MDrakos Feb 14, 2024

Naatan Feb 14, 2024

MDrakos Feb 14, 2024

Naatan left a comment

Naatan Feb 14, 2024

MDrakos Feb 14, 2024

Naatan Feb 15, 2024

MDrakos Feb 15, 2024 •

edited

Loading

MDrakos commented Feb 14, 2024

mitchell-as commented Feb 15, 2024

Naatan commented Feb 15, 2024

Naatan Feb 15, 2024

MDrakos Feb 15, 2024

Naatan Feb 15, 2024

Naatan left a comment

MDrakos commented Feb 15, 2024

Naatan left a comment

Clean up state search UI #3099

Clean up state search UI #3099

Conversation

MDrakos commented Feb 11, 2024 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Naatan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MDrakos Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

MDrakos commented Feb 14, 2024

mitchell-as commented Feb 15, 2024

Naatan commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Naatan left a comment

Choose a reason for hiding this comment

MDrakos commented Feb 15, 2024

Naatan left a comment

Choose a reason for hiding this comment

Clean up `state search` UI #3099

Clean up `state search` UI #3099

MDrakos commented Feb 11, 2024 •

edited by github-actions bot

Loading

MDrakos Feb 15, 2024 •

edited

Loading