Skip to content

Enable Shortcuts to Comment Lines in Query Editor#10545

Merged
angle943 merged 2 commits intoopensearch-project:mainfrom
LDrago27:addCommentShortcut
Sep 20, 2025
Merged

Enable Shortcuts to Comment Lines in Query Editor#10545
angle943 merged 2 commits intoopensearch-project:mainfrom
LDrago27:addCommentShortcut

Conversation

@LDrago27
Copy link
Copy Markdown
Collaborator

@LDrago27 LDrago27 commented Sep 19, 2025

Description

This enables Shortcuts to Comment Lines in Query Editor.

  • Use the shortcuts cmd + / to comment/uncomment a line
  • Use the shortcuts shift+ option + / to comment/uncomment a selected block

Issues Resolved

Screenshot

Testing the changes

Meeting.Recording.-.Sahoo.Suchit.Instant.Meeting.28.mp4

Changelog

  • feat: Enable Shortcuts to Comment Lines in Query Editor

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Suchit Sahoo <suchsah@amazon.com>
@github-actions
Copy link
Copy Markdown
Contributor

❌ Invalid Prefix

Invalid description prefix. Found "feat ". Expected "breaking", "deprecate", "feat", "fix", "infra", "doc", "chore", "refactor", "security", "skip", or "test".

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.14%. Comparing base (87a48be) to head (ee4701c).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10545      +/-   ##
==========================================
- Coverage   62.14%   62.14%   -0.01%     
==========================================
  Files        4299     4299              
  Lines      110328   110328              
  Branches    18199    18199              
==========================================
- Hits        68565    68563       -2     
- Misses      37083    37085       +2     
  Partials     4680     4680              
Flag Coverage Δ
Linux_1 27.64% <ø> (ø)
Linux_2 41.48% <ø> (ø)
Linux_3 39.50% <ø> (ø)
Linux_4 33.85% <ø> (-0.01%) ⬇️
Windows_1 27.65% <ø> (ø)
Windows_2 41.45% <ø> (ø)
Windows_3 39.51% <ø> (ø)
Windows_4 33.85% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

comments: {
lineComment: '//', // line comment
blockComment: ['/*', '*/'], // block comment
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you see if there was a test that checks for these other stuff? if there was, might be worth adding this one too so that test fails if someone does something to it by accidently in the future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think there is a test for it right now. We can add a cypress test for it.

@LDrago27
Copy link
Copy Markdown
Collaborator Author

Will merge this once the Grammar upgrade is merged in. #10536 . Since without the grammar upgrade the parser won't recognize the comments as valid PPL token.

@angle943 angle943 merged commit d9ab3bb into opensearch-project:main Sep 20, 2025
82 of 83 checks passed
@angle943 angle943 added the OSD Changes being merged by the OSD team label Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

distinguished-contributor OSD Changes being merged by the OSD team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants