From 71f694ac56e9878602f9f0f5b3ea73a5cf2cd9d4 Mon Sep 17 00:00:00 2001 From: Felix Habib <33821218+felixhabib@users.noreply.github.com> Date: Wed, 6 Mar 2024 15:06:27 +1100 Subject: [PATCH 1/2] Refactor logic to determine comment type for "Toggle comment" command (#340) Co-authored-by: Ben Jervis --- .changeset/many-trains-cheat.md | 5 ++ cypress/e2e/keymaps.cy.js | 72 +++++++++++++++++++++- src/Playroom/CodeEditor/keymaps/comment.ts | 21 ++++++- 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 .changeset/many-trains-cheat.md diff --git a/.changeset/many-trains-cheat.md b/.changeset/many-trains-cheat.md new file mode 100644 index 00000000..9e9eeaa0 --- /dev/null +++ b/.changeset/many-trains-cheat.md @@ -0,0 +1,5 @@ +--- +'playroom': patch +--- + +Fix issue with "Toggle comment" command commenting certain code outside JSX tags with incorrect syntax. diff --git a/cypress/e2e/keymaps.cy.js b/cypress/e2e/keymaps.cy.js index 620ded93..1198ce4b 100644 --- a/cypress/e2e/keymaps.cy.js +++ b/cypress/e2e/keymaps.cy.js @@ -412,7 +412,7 @@ describe('Keymaps', () => { > Text - `); + `); moveBy(0, 2); @@ -429,6 +429,76 @@ describe('Keymaps', () => { `); }); + + it('block - expression slot outside tags', () => { + loadPlayroom(` + {testFn('test')} +
First line
+
Second line
+
Third line
+ `); + + executeToggleCommentCommand(); + + assertCodePaneContains(dedent` + {/* {testFn('test')} */} +
First line
+
Second line
+
Third line
+ `); + }); + + it('line - inside multi-line expression slot outside tags', () => { + loadPlayroom(` + { + testFn('test') + } +
First line
+
Second line
+
Third line
+ `); + + moveBy(0, 1); + + executeToggleCommentCommand(); + + assertCodePaneContains(dedent` + { + // testFn('test') + } +
First line
+
Second line
+
Third line
+ `); + }); + + it('line - full line expression slot inside tags', () => { + loadPlayroom(` +
+ First line +
+
Second line
+
Third line
+ `); + + moveBy(0, 2); + + executeToggleCommentCommand(); + + assertCodePaneContains(dedent` +
+ First line +
+
Second line
+
Third line
+ `); + }); }); describe('should wrap a single line selection in a comment', () => { diff --git a/src/Playroom/CodeEditor/keymaps/comment.ts b/src/Playroom/CodeEditor/keymaps/comment.ts index 3da03d3e..b963f401 100644 --- a/src/Playroom/CodeEditor/keymaps/comment.ts +++ b/src/Playroom/CodeEditor/keymaps/comment.ts @@ -198,6 +198,19 @@ function getUpdatedContent(existingContent: string, range: TagRange) { type CommentType = 'line' | 'block'; +const isOnlyWhitespace = (input: string) => /^\s+$/.test(input); + +const isFullExpressionSlot = (tokens: CodeMirror.Token[]) => { + const formattedLineTokens = tokens.filter( + (token) => token.type !== 'comment' && !isOnlyWhitespace(token.string) + ); + + return ( + formattedLineTokens.at(0)?.string === '{' && + formattedLineTokens.at(-1)?.string === '}' + ); +}; + interface TagRange { from: CodeMirror.Position; to: CodeMirror.Position; @@ -219,7 +232,9 @@ const determineCommentType = ( (token) => token.type === 'attribute' ); - const isJavaScriptMode = cm.getModeAt(from).name === 'javascript'; + const isJavaScriptMode = + cm.getModeAt(new Pos(from.line, 0)).name === 'javascript'; + const isInlineComment = cm .getLine(from.line) .trimStart() @@ -228,6 +243,10 @@ const determineCommentType = ( cm.getLine(from.line).trimStart().startsWith(BLOCK_COMMENT_START) && cm.getLine(to.line).trimEnd().endsWith(BLOCK_COMMENT_END); + if (!isBlockComment && isFullExpressionSlot(lineTokens)) { + return isJavaScriptMode ? 'block' : 'line'; + } + if (isInlineComment) { return 'line'; } From 9208310fec2c168bcac5b6566f45ecc006fd26ff Mon Sep 17 00:00:00 2001 From: Felix Habib <33821218+felixhabib@users.noreply.github.com> Date: Wed, 6 Mar 2024 15:59:27 +1100 Subject: [PATCH 2/2] Revert "Replace `concurrently` dev dep with `pnpm run` (#339)" to fix Cypress locally (#342) --- package.json | 9 +++++---- pnpm-lock.yaml | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index a122c3c4..9e7b05bc 100644 --- a/package.json +++ b/package.json @@ -20,11 +20,11 @@ "start:typescript": "./bin/cli.cjs start --config cypress/projects/typescript/playroom.config.js", "build:typescript": "./bin/cli.cjs build --config cypress/projects/typescript/playroom.config.js", "serve:typescript": "PORT=9002 serve --no-request-logging cypress/projects/typescript/dist", - "start:all": "pnpm run '/^start:(?!all)/'", - "build:all": "pnpm run '/^build:(?!all)/'", - "serve:all": "pnpm run '/^serve:(?!all)/'", + "start:all": "concurrently 'npm:start:*(!all)'", + "build:all": "concurrently 'npm:build:*(!all)'", + "serve:all": "concurrently 'npm:serve:*(!all)'", "build-and-serve:all": "pnpm build:all && pnpm serve:all", - "lint": "pnpm run '/^lint:.*/'", + "lint": "concurrently 'npm:lint:*'", "lint:eslint": "eslint --cache .", "lint:prettier": "prettier --list-different '**/*.{js,md,ts,tsx}'", "lint:tsc": "tsc --noEmit", @@ -115,6 +115,7 @@ "@octokit/rest": "^19.0.5", "@types/jest": "^29.2.4", "@types/react-helmet": "^6.1.6", + "concurrently": "^7.6.0", "cypress": "^13.6.6", "eslint": "^8.44.0", "eslint-config-seek": "^11.3.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ffa79eb2..d44b2747 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -181,6 +181,9 @@ devDependencies: '@types/react-helmet': specifier: ^6.1.6 version: 6.1.11 + concurrently: + specifier: ^7.6.0 + version: 7.6.0 cypress: specifier: ^13.6.6 version: 13.6.6 @@ -4254,6 +4257,22 @@ packages: /concat-map@0.0.1: resolution: {integrity: sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==} + /concurrently@7.6.0: + resolution: {integrity: sha512-BKtRgvcJGeZ4XttiDiNcFiRlxoAeZOseqUvyYRUp/Vtd+9p1ULmeoSqGsDA+2ivdeDFpqrJvGvmI+StKfKl5hw==} + engines: {node: ^12.20.0 || ^14.13.0 || >=16.0.0} + hasBin: true + dependencies: + chalk: 4.1.2 + date-fns: 2.29.3 + lodash: 4.17.21 + rxjs: 7.6.0 + shell-quote: 1.8.1 + spawn-command: 0.0.2-1 + supports-color: 8.1.1 + tree-kill: 1.2.2 + yargs: 17.6.2 + dev: true + /connect-history-api-fallback@2.0.0: resolution: {integrity: sha512-U73+6lQFmfiNPrYbXqr6kZ1i1wiRqXnp2nhMsINseWXO8lDau0LGEffJ8kQi4EjLZympVgRdvqjAgiZ1tgzDDA==} engines: {node: '>=0.8'} @@ -4462,6 +4481,11 @@ packages: assert-plus: 1.0.0 dev: true + /date-fns@2.29.3: + resolution: {integrity: sha512-dDCnyH2WnnKusqvZZ6+jA1O51Ibt8ZMRNkDZdyAyK4YfbDwa/cEmuztzG5pk6hqlp9aSBPYcjOlktquahGwGeA==} + engines: {node: '>=0.11'} + dev: true + /dayjs@1.11.7: resolution: {integrity: sha512-+Yw9U6YO5TQohxLcIkrXBeY73WP3ejHWVvx8XCk3gxvQDCTEmS48ZrSZCKciI7Bhl/uCMyxYtE9UqRILmFphkQ==} dev: true @@ -8937,7 +8961,6 @@ packages: /shell-quote@1.8.1: resolution: {integrity: sha512-6j1W9l1iAs/4xYBI1SYOVZyFcCis9b4KCLQ8fgAGG07QvzaRLVVRQvAy85yNmmZSjYjg4MWh4gNvlPujU/5LpA==} - dev: false /side-channel@1.0.4: resolution: {integrity: sha512-q5XPytqFEIKHkGdiMIrY10mvLRvnQh42/+GoBlFW3b2LXLE2xxJpZFdm94we0BaoV3RwJyGqg5wS7epxTv0Zvw==} @@ -9055,6 +9078,10 @@ packages: deprecated: Please use @jridgewell/sourcemap-codec instead dev: false + /spawn-command@0.0.2-1: + resolution: {integrity: sha512-n98l9E2RMSJ9ON1AKisHzz7V42VDiBQGY6PB1BwRglz99wpVsSuGzQ+jOi6lFXBGVTCrRpltvjm+/XA+tpeJrg==} + dev: true + /spawndamnit@2.0.0: resolution: {integrity: sha512-j4JKEcncSjFlqIwU5L/rp2N5SIPsdxaRsIv678+TZxZ0SRDJTm8JrxJMjE/XuiEZNEir3S8l0Fa3Ke339WI4qA==} dependencies: @@ -9610,6 +9637,11 @@ packages: resolution: {integrity: sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==} dev: true + /tree-kill@1.2.2: + resolution: {integrity: sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A==} + hasBin: true + dev: true + /trim-newlines@3.0.1: resolution: {integrity: sha512-c1PTsA3tYrIsLGkJkzHF+w9F2EyxfXGo4UyJc4pFL++FMjnq0HJS69T3M7d//gKrFKwy429bouPescbjecU+Zw==} engines: {node: '>=8'}