feat(launchpad): open in IDE modal and feature #18975

lmiller1990 · 2021-11-18T09:09:25Z

Features

Configure preferred IDE from launchpad and app if one isn't configured
Open file in preferred IDE
Open project directory in preferred IDE
Open project directory in finder

Testing

See videos - they show the feature, where it is, how it works. Be good to get someone else to give it a test, too. I also added a bunch of e2e tests.

More Info

I changed up how reporter communicates w/ event manager and the back end when opening files. Reporter is now less smart - it just emits an event, and the runner will handle the request to open a file via GraphQL. This made the React -> Vue -> event manager interop much more simple and avoided a bunch of hacks, as well as made the types better, and allowed for better e2e tests.

Prevously, the front-end passed the preferred editor with the "open in file" request, which I don't think is ideal. We should only have one source of truth for the preferred editor, and that should be on the server, not in the front-end.

I left in the existing "open in file" functionality since removing or changing it breaks a lot of system tests.

Demos

Demo: set IDE and open from app

app-open-demo.mov

Demo: set IDE and open from launchpad

launchpad-open-demo.mov

cypress-bot · 2021-11-18T09:09:28Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2021-11-18T09:26:34Z

Test summary

18698 • 1 • 204 • 0 • 3

Run details


Project	cypress
Status	Failed
Commit	`0a57c91`
Started	Nov 24, 2021 12:48 AM
Ended	Nov 24, 2021 1:00 AM
Duration	12:05 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Failures

	cypress/integration/commands/net_stubbing_spec.ts		1
1	network stubbing > intercepting request > should delay the same amount on every response

Flakiness

	commands/net_stubbing_spec.ts	2
1	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
2	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
	cypress/proxy-logging-spec.ts	1
1	... > works with forceNetworkError

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

…pen-in-ide-UNIFY-593

lmiller1990 · 2021-11-19T07:47:14Z

packages/app/src/pages/Runner.vue

 }
 `

 const query = useQuery({ query: SpecPageContainerDocument })
+
+export type GqlWithCurrentProject = SpecPageContainerQuery & {


This feels hacky, @tgriesser can we sync on this? Also notified in slack

elevatebart

I wonder if it was intended but when testing this feature, I found an issue.

If you set your editor in the settings page using the drop-down, wait a second and then refresh the page, the value is empty again.

It looks like GQL is not aware of the mutation and does not invalidate the needed query's cache.

@tgriesser @estrada9166 maybe you can help with this cache issue?

If it is already investigated and solved somewhere else, I withdraw my comment. but it is annoying.

elevatebart · 2021-11-19T16:30:23Z

packages/frontend-shared/src/gql-components/ChooseExternalEditor.vue

+import VSCode from '~icons/logos/visual-studio-code'
+import Atom from '~icons/logos/atom-icon'
+import Webstorm from '~icons/logos/webstorm'
+import Vim from '~icons/logos/vim'
+import Sublime from '~icons/logos/sublimetext-icon'
+import Emacs from '~icons/logos/emacs'
+import IconTerminal from '~icons/mdi/terminal'


Why are those icons not part of the env-editors.ts ?

I get it, because they are not available there.

Maybe we could use import meta to import a glob of logos and only reference logos by name on the server...
https://vitejs.dev/guide/features.html#glob-import

This is an architecture concern though.

Could we at least add one icon corresponding to computer for the open in file system choice.

And use the terminal icon from cypress instead of MDI?

Done and done for icons, let's consider the Vite globbing, too, at some point...

…pen-in-ide-UNIFY-593

lmiller1990 · 2021-11-22T03:08:29Z

@elevatebart

I wonder if it was intended but when testing this feature, I found an issue.

I made a change in a commit and looks like I didn't test it - setting an editor on that page was completely not working as it should have been. Huge oversight on my end, extended the e2e test to make sure it does not regress: 4ec6bbf

elevatebart

Except for some icons not aligned it works

elevatebart · 2021-11-22T14:53:47Z

packages/frontend-shared/src/gql-components/ChooseExternalEditor.vue

+import VSCode from '~icons/logos/visual-studio-code'
+import Atom from '~icons/logos/atom-icon'
+import Webstorm from '~icons/logos/webstorm'
+import Vim from '~icons/logos/vim'
+import Sublime from '~icons/logos/sublimetext-icon'
+import Emacs from '~icons/logos/emacs'
+import IconTerminal from '~icons/mdi/terminal'


elevatebart · 2021-11-23T17:47:38Z

packages/frontend-shared/cypress/e2e/support/e2eProjectDirs.ts

@@ -65,6 +65,7 @@ export const e2eProjectDirs = [
  'screen-size',
  'shadow-dom-global-inclusion',
  'spec-generation',
+  'spec-name-special-characters',


Why is this file updated?
@tgriesser should we gitignore it?

ZachJW34

Tested, looks good. I'm curious about the comment you made about the hacky GraphQL types. Can we not just use the normal fragment types here? Might require some refactoring of the fragments though

elevatebart · 2021-11-23T20:39:42Z

packages/data-context/src/actions/FileActions.ts

+    if (!binary || !absolute) {
+      this.ctx.debug('cannot open file without binary')
+
+      return
+    }
+
+    if (binary === 'computer') {
+      try {
+        this.ctx.electronApi.showItemInFolder(absolute)
+      } catch (err) {
+        this.ctx.debug('error opening file: %s', (err as Error).stack)
+      }
+
+      return
+    }
+
+    launchEditor(`${absolute}:${line}:${column}`, `"${binary}"`, (__: unknown, errMsg: string) => {
+      this.ctx.debug('error opening file: %s', errMsg)
+    })


Do you think this implementation should be done here or could it be done in another file?

I guess, why? Where?

lmiller1990 · 2021-11-24T00:20:45Z

@ZachJW34 I summed up about the types here: https://cypressio.slack.com/archives/C027U3ZEJ6Q/p1637307934004600 basically in GraphQL currentProject is nullable, but on this page it will never be null - I want some way to express that.

For now I reverted the hack and just added some null checks. Long term we will wait for the ! operator on fields to land in urql: urql-graphql/urql#2018

Then we can do

query Q {
  currentProject!
}

... represents that the client expects currentProject not to be null.

* 10.0-release: refactor: remove @packages/desktop-gui (#19127) feat: switch browser runner (#19048) fix: bump resource class for tests that require node_modules install (#19079) fix(unify): reporter styles (#19034) fix test feat(unify): add number of matches to specs search (#19076) feat(launchpad): open in IDE modal and feature (#18975) fix: 10.0 appveyor updateyaml (#19074)

lmiller1990 added 2 commits November 18, 2021 18:30

better tests

8f21349

basic support for open in ide

b9a33d4

lmiller1990 added 14 commits November 19, 2021 11:10

Merge remote-tracking branch 'origin/10.0-release' into lmiller1990/o…

071e1d9

…pen-in-ide-UNIFY-593

Merge remote-tracking branch 'origin/10.0-release' into lmiller1990/o…

867c1e2

…pen-in-ide-UNIFY-593

extract modal component

c93830a

mutation to open file

0c4d38b

remove old code

9c31e74

add a ton of tests

e337d40

update

a25b341

lint

de0cd0a

types

4e899d5

ignore typeless import

d2a8dd5

update links

adcefe6

backwards compat so we do not break all system tests

d8d3b6f

add back missing code

c51f265

missing code

c4cf529

lmiller1990 marked this pull request as ready for review November 19, 2021 07:39

lmiller1990 requested a review from a team as a code owner November 19, 2021 07:39

lmiller1990 requested review from jennifer-shehane, a team and tgriesser and removed request for a team and jennifer-shehane November 19, 2021 07:39

lmiller1990 commented Nov 19, 2021

View reviewed changes

elevatebart suggested changes Nov 19, 2021

View reviewed changes

elevatebart reviewed Nov 19, 2021

View reviewed changes

lmiller1990 added 3 commits November 22, 2021 12:01

Merge remote-tracking branch 'origin/10.0-release' into lmiller1990/o…

f35eba9

…pen-in-ide-UNIFY-593

fix bug

b9fbb7d

add regression test

4ec6bbf

ts

a76fe8f

elevatebart previously approved these changes Nov 22, 2021

View reviewed changes

tgriesser dismissed elevatebart’s stale review via a76fe8f November 22, 2021 18:16

elevatebart reviewed Nov 23, 2021

View reviewed changes

ZachJW34 previously approved these changes Nov 23, 2021

View reviewed changes

elevatebart reviewed Nov 23, 2021

View reviewed changes

merge in origin

86621c7

lmiller1990 dismissed ZachJW34’s stale review via 86621c7 November 24, 2021 00:22

types

0a57c91

ZachJW34 approved these changes Nov 24, 2021

View reviewed changes

lmiller1990 merged commit 168600b into 10.0-release Nov 24, 2021

lmiller1990 deleted the lmiller1990/open-in-ide-UNIFY-593 branch November 24, 2021 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(launchpad): open in IDE modal and feature #18975

feat(launchpad): open in IDE modal and feature #18975

lmiller1990 commented Nov 18, 2021 •

edited by elevatebart

Loading

cypress-bot bot commented Nov 18, 2021

cypress bot commented Nov 18, 2021 •

edited

Loading

lmiller1990 Nov 19, 2021

elevatebart left a comment •

edited

Loading

elevatebart Nov 19, 2021

elevatebart Nov 19, 2021 •

edited

Loading

elevatebart Nov 19, 2021

lmiller1990 Nov 22, 2021

elevatebart Nov 22, 2021

lmiller1990 commented Nov 22, 2021

elevatebart left a comment

elevatebart Nov 22, 2021

elevatebart Nov 23, 2021

ZachJW34 left a comment

elevatebart Nov 23, 2021

lmiller1990 Nov 24, 2021

lmiller1990 commented Nov 24, 2021 •

edited

Loading

feat(launchpad): open in IDE modal and feature #18975

feat(launchpad): open in IDE modal and feature #18975

Conversation

lmiller1990 commented Nov 18, 2021 • edited by elevatebart Loading

Features

Testing

More Info

Demos

cypress-bot bot commented Nov 18, 2021

cypress bot commented Nov 18, 2021 • edited Loading

Test summary

Run details

Failures

Flakiness

Choose a reason for hiding this comment

elevatebart left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elevatebart Nov 19, 2021 • edited 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

lmiller1990 commented Nov 22, 2021

elevatebart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZachJW34 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented Nov 24, 2021 • edited Loading

lmiller1990 commented Nov 18, 2021 •

edited by elevatebart

Loading

cypress bot commented Nov 18, 2021 •

edited

Loading

elevatebart left a comment •

edited

Loading

elevatebart Nov 19, 2021 •

edited

Loading

lmiller1990 commented Nov 24, 2021 •

edited

Loading