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

Handle OnEnterRule patterns #12228

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

gbodeen
Copy link
Contributor

@gbodeen gbodeen commented Feb 24, 2023

What it does

Fixes #12217 and the issue raised in #12212 .

The format of OnEnterRule had been defined with beforeText, afterText, and previousLineText as only strings. However they are often quasi-RegExp objects with pattern and flags string fields.

How to test

  1. With TypeScript plugins 1.66.2 or later, in a JavaScript or TypeScript file, if you hit Enter to create a new line, do you still get * appended inappropriately?
  2. Similarly in HTML files, if you hit Enter to create a new line, do you still get two new lines inappropriately?
  3. Do other insertions (e.g. for JsDoc strings) still work properly?

Review checklist

Note: I've only tested on RHEL7, electron & browser modes, and a limited set of circumstances involving insertions of newlines.

Reminder for reviewers

@colin-grant-work colin-grant-work added plug-in system issues related to the plug-in system languages issue related to languages labels Feb 24, 2023
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

These changes address the problems previously observed with recent language feature plugins. 👍

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍
I confirmed that:

  • the onEnterRule are properly applied (ex: comments)
  • we no longer get the odd behavior for HTML, or TypeScript for newlines

@colin-grant-work colin-grant-work merged commit e2f76b6 into eclipse-theia:master Feb 27, 2023
@colin-grant-work colin-grant-work added this to the 1.36.0 milestone Feb 27, 2023
@ccorn
Copy link

ccorn commented Feb 28, 2023

This means that 2f287df can be reverted, right?

@vince-fugnitto
Copy link
Member

@ccorn the changes already remove the pinned builtins:

https://github.com/eclipse-theia/theia/pull/12228/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L106-R106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
languages issue related to languages plug-in system issues related to the plug-in system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Newlines are doubled in HTML
4 participants