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

ContextFactory reused to initialize language-server context #10670

Merged
merged 9 commits into from
Jul 29, 2024

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jul 25, 2024

Pull Request Description

Move ContextFactory to common module, so it is also available in language-server module and can be used there as well. As a result the initialization of experimental Espresso support is now unified. Otherwise this PR is just a refactoring except a single change: when using --inspect and --run hitting Standard.Base.Runtime.Debug.breakpoint statement, the debugger stops in Chrome Dev Tools, as the internal debug server is disabled.

Checklist

  • All code follows the
    Scala,
    Java,
  • Unit tests continue to work

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 25, 2024
@JaroslavTulach JaroslavTulach self-assigned this Jul 25, 2024
@JaroslavTulach JaroslavTulach marked this pull request as draft July 25, 2024 10:56
@JaroslavTulach JaroslavTulach requested a review from radeusgd July 25, 2024 10:58
@JaroslavTulach JaroslavTulach changed the title Moving ContextFactory to org.enso.common Common ContextFactory to also initialize language-server context Jul 25, 2024
@JaroslavTulach JaroslavTulach changed the title Common ContextFactory to also initialize language-server context ContextFactory reused to initialize language-server context Jul 25, 2024
@JaroslavTulach JaroslavTulach marked this pull request as ready for review July 26, 2024 08:24
.options(options);

if (inspect) {
options.put("inspect", "");
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 realized that when one uses --inspect and hit Debug.breakpoint statement we actually hit the breakpoint twice. Once in Chrome Dev Tools and once in REPL. I believe it is better to have just a single debugger running. As such I am proposing to disable debug server when --inspect option is used.

Copy link
Member

Choose a reason for hiding this comment

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

That definitely makes sense.

Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

Looks good

.allowAllAccess(true)
.allowHostAccess(new HostAccessFactory().allWithTypeMapping())
.option(RuntimeOptions.PROJECT_ROOT, pkg.root.getCanonicalPath)
.option("js.foreign-object-prototype", "true")
Copy link
Contributor

Choose a reason for hiding this comment

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

This option is no longer needed?

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 don't know what that option is good for?

In any case, it is not used by CLI. I see two PRs that refer to that option:

There were no failed tests when the option was removed, so it is not essential for any use-case we test for.

@JaroslavTulach JaroslavTulach merged commit cb72487 into develop Jul 29, 2024
41 checks passed
@JaroslavTulach JaroslavTulach deleted the wip/jtulach/ContextFactoryCommon branch July 29, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants