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

Change guidance that mentions fuse when there's multiple shapes #3150

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

philrz
Copy link
Contributor

@philrz philrz commented Sep 4, 2024

tl;dr

When results contain multiple shapes, I'm proposing changing the wording of the guidance in the default Table view to:

N Shapes — Filter or fuse to view results as one shape.

Details

This is one that hit me when I was testing the changes in #3145.

Right now when the results contain multiple shapes, the default Table view shows the guidance:

N Shapes — Filter to one shape or fuse results to view as a table.

This guidance is accurate if the data happens to be records, which it often is. However (and maybe this is spitting hairs), if the data contains one or more primitive values, such as this ZSON test data:

1
"hello"
3.14

the guidance becomes inaccurate because even if the user does apply fuse, the data shown is still not in a "table" in the traditional sense because there's no header containing a field name. Rather, the data is instead still just a sequence of values, though now fuse has applied a union type to all of them such that the user is, indeed, now looking at a single shape.

Therefore, in this PR I've proposed adjusting the guidance to instead say:

N Shapes — Filter or fuse to view results as one shape.

since this seems to cover all cases in a technically accurate way.

(Thanks to @nwt for the proposed wording!)

image

(Really, I think the most important part of this guidance has always been giving the user the one-click access to fuse, since this is probably what users are looking for much of the time, but new users in particular are unlikely to immediately read through the Zed docs and find it right off the bat. All that is still fully intact with this change, so hopefully it's non-controversial.)

@philrz philrz self-assigned this Sep 4, 2024
Copy link
Member

@jameskerr jameskerr left a comment

Choose a reason for hiding this comment

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

Love it. Thank you for the PR

@philrz philrz merged commit e54d07e into main Sep 5, 2024
4 checks passed
@philrz philrz deleted the change-fuse-guidance branch September 5, 2024 20:51
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.

2 participants