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

DDP-7226 PEX editor #977

Merged
merged 18 commits into from
Jan 28, 2022
Merged

DDP-7226 PEX editor #977

merged 18 commits into from
Jan 28, 2022

Conversation

aglushko-broad
Copy link
Contributor

@aglushko-broad aglushko-broad commented Dec 10, 2021

Pex editor component is integrated in study-builder-ui project.
Route for pex-editor /pex-sandbox.

List of implemented features:

  • validations (list of errors is shown under editor component)
  • syntax highlighting (can be improved, didn't have much time to pick the perfect colors for different tokens)
  • suggestions+code completion (works with issues for predicates which takes multiple strings as argumens, e.g. hasAnyOption)

quick start: run npm i, then run antlr4ts Pex.g4 from src/antlr4-pex-grammar folder
image
image

@aglushko-broad aglushko-broad marked this pull request as draft December 10, 2021 13:17
@aglushko-broad aglushko-broad marked this pull request as ready for review December 29, 2021 14:39
Anastasiia_Glushkova and others added 3 commits December 30, 2021 19:22
@alextanlan
Copy link
Contributor

I added some small changes. Also added generated files (PexLexer, PexParser). The files should be re-generated when Pex grammar file is changed.

@alextanlan
Copy link
Contributor

Generally, looks pretty good! Lots of work is done.
However, several requirements from the ticket DDP-7226 are not implemented yet:

  • there is not a validation for parameters (e.g. we can not check a wrong study name)

    1.b. If the wrong study name (in example above it is “pancan” somebody enters “pancan’t” we would again, at least detect
    that there is an error in the expression, and preferably, the exact character position of the error.

  • we don't have suggestions for parameters (e.g. question stable ids):

    2.b. When cursor is placed inside of the [] after questions, all the possible question stable ids are offered for completion.

  • Rare invalid cases are possible. For example using parameters for predicates which are not allowed to have parameters.

    user.profile.age("ANY STRING")

    Or invalid order of a query.

    user.profile.language().age().language.profile.age.profile

@alextanlan
Copy link
Contributor

My proposal:
Get the PR merged. And then if/when we need more improvements - continue with it.

@MocanaAtBroad, what do you think about it ?

alextanlan
alextanlan previously approved these changes Jan 11, 2022
# Conflicts:
#	ddp-workspace/package-lock.json
#	ddp-workspace/package.json
@MocanaAtBroad
Copy link
Contributor

Looks like we might have to do a little bit of work to update editor to Angular 13? Error below. I was able to run ng serve by running npm install --force, but probably not a good solution when we want to use this for real.
Screen Shot 2022-01-23 at 1 34 40 PM

@MocanaAtBroad
Copy link
Contributor

The things the editor can do at this point are pretty cool! I think we want to try to wrap up the work that has been done so that we can pick it up again later when we are done with FON.
My suggestion at this point:

  1. See if we can figure out the dependency problem I mentioned above.
  2. Create a JIRA ticket with a list of issues/problems/fixes that need to be addressed to complete this component.
  3. Merge and close this ticket.
    Other ideas?

# Conflicts:
#	ddp-workspace/projects/ddp-sdk/src/lib/ddp.module.ts
('ERESOLVE unable to resolve dependency tree')

  `by  npm i --legacy-peer-deps`
@alextanlan
Copy link
Contributor

alextanlan commented Jan 24, 2022

We might have to do a little bit of work to update editor to Angular 13? See if we can figure out the dependency problem above.

The Pex editor already deals with Angular 13. And the dependency issue is not related to Angular at all.
That is the latest npm version issue ERESOLVE unable to resolve dependency tree
(most likely, even a bug - npm issue 1, npm issue 2).

Npm (version >= 7) installs peerDependencies by default automatically. Seems like in many cases, this is leading to version conflicts, which will break the installation process. A "conflicting peer dependency" error is that some package we depend upon is expressing a peer dependency on a package version spec which does not match the version of that package that I actually have installed.

In our case we have ngx-monaco-editor version conflict.

The weirdest thing that I can't even install dependencies with npm i --force on my Windows machine. There are lots of errors (
errors log file).

I found out another workaround.
Using npm i --legacy-peer-deps restores peerDependency installation behavior like with npm v4-6.
Seems it works properly in our case.

@alextanlan
Copy link
Contributor

Create a JIRA ticket with a list of issues/problems/fixes that need to be addressed to complete this component.

Created DDP-7482

josemanuel167
josemanuel167 previously approved these changes Jan 27, 2022
# Conflicts:
#	ddp-workspace/angular.json
#	ddp-workspace/projects/study-builder-ui/src/app/app.module.ts
@alextanlan alextanlan merged commit d3fb0d3 into develop Jan 28, 2022
@alextanlan alextanlan deleted the DDP-7226-pex-editor branch January 28, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants