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

Fix possibly cuttoff cli errors in tree views #4522

Merged
merged 7 commits into from
Aug 18, 2023
Merged

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Aug 16, 2023

  • fix errors in tree views possibly getting cut off due to containing a forward slash

Main

Screenshot 2023-08-16 at 5 47 22 PM

PR

Screenshot 2023-08-16 at 5 47 57 PM

Fixes #4519

@julieg18 julieg18 added the bug Something isn't working label Aug 16, 2023
@julieg18 julieg18 self-assigned this Aug 16, 2023
@julieg18 julieg18 marked this pull request as ready for review August 16, 2023 23:23
@@ -61,6 +61,9 @@ export const getCliErrorTreeItem = (
command: RegisteredCommands.EXTENSION_SHOW_OUTPUT,
title: 'Show DVC Output'
}

treeItem.label = label
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the issue has something to do with the fact that we are creating a uri with the error message in the tree item which expects a file path 🤔

extension/src/tree/index.ts Show resolved Hide resolved
extension/src/tree/index.ts Outdated Show resolved Hide resolved
@@ -406,12 +407,16 @@ describe('RepositoriesTree', () => {

expect(mockedTreeItem).toHaveBeenCalledTimes(1)
expect(treeItem).toStrictEqual({
...mockedItem,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tests for each tree that uses the cliError tree item and adjusted the check to not use mockedItem so we could confirm that label is or is not there.

Copy link
Contributor Author

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Simplified the solution and added tests!

@julieg18 julieg18 enabled auto-merge (squash) August 18, 2023 12:47
@codeclimate
Copy link

codeclimate bot commented Aug 18, 2023

Code Climate has analyzed commit 6c0c00b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.2% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 1fdb225 into main Aug 18, 2023
3 checks passed
@julieg18 julieg18 deleted the fix-cutoff-cli-errors branch August 18, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experiments UI shows cutoff error message
2 participants