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

Use builtin vscode.open command in tracked explorer tree #807

Merged
merged 4 commits into from
Sep 14, 2021

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Sep 14, 2021

2/2 master <- #806 <- this

As detailed in the description of #806 there was a better way of opening files from the tracked explorer tree.

This PR uses the vscode builtin vscode.open command to open files from the tree with the left click mouse action. I have also deleted all of the code that we no longer need as a result.

Demo:

Screen.Recording.2021-09-14.at.2.23.43.pm.mov
Screen.Recording.2021-09-14.at.2.25.53.pm.mov

For context the history of this goes back to #528 (and beyond).

@mattseddon mattseddon added the product PR that affects product label Sep 14, 2021
@mattseddon mattseddon self-assigned this Sep 14, 2021
"description": "%config.noOpenUnsupported.description%",
"type": "boolean",
"default": false
},
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] We no longer need this option which is good.

Copy link
Member Author

Choose a reason for hiding this comment

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

I previously added "Don't Show Again" options for some of our (more frequent / annoying) toast popups as per the notifications section of the extension guidelines

@mattseddon mattseddon changed the base branch from master to add-open-to-the-side September 14, 2021 04:27
error => {
if (
error.message.includes(
'File seems to be binary and cannot be opened as text'
Copy link
Member Author

Choose a reason for hiding this comment

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

[C] Not relying on free text from an error message anymore, also good.

@mattseddon mattseddon marked this pull request as ready for review September 14, 2021 04:29
expect(mockPull).not.to.be.called
expect(
mockShowInformationMessage,
'should show the user an information prompt'
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] The only difference with this test is the added expect breadcrumb. They should make it easier to grok what is going on next time we come round.

Base automatically changed from add-open-to-the-side to master September 14, 2021 05:01
@mattseddon mattseddon force-pushed the improve-tracked-explorer-open branch from aa1a8b2 to e2628b9 Compare September 14, 2021 05:03
@mattseddon mattseddon enabled auto-merge (squash) September 14, 2021 05:04
@codeclimate
Copy link

codeclimate bot commented Sep 14, 2021

Code Climate has analyzed commit e2628b9 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 96.0% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit e1bf948 into master Sep 14, 2021
@mattseddon mattseddon deleted the improve-tracked-explorer-open branch September 14, 2021 05:07
@mattseddon
Copy link
Member Author

related to #569

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants