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

Add more services to statemachine example #1265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Yokozuna59
Copy link
Contributor

@Yokozuna59 Yokozuna59 commented Nov 4, 2023

Resolves #1257.

  • validation

    • Validate unreachable states
    • Validate unused events
  • code actions

    • If a state name doesn't start with a capital letter, replace it with capitalized name (look at the corresponding validation here)
    • Remove an unreachable state
    • Remove an unused event
  • LSP

    • Formatter
      * [ ] RenameProvider (it's working fine)

I also validated commands with events and states.

I haven't implemented the full version of quick fix to make state name starts with capital letter yet. The current Langium LSP didn't implement such feature yet, so I'm not sure how to do it in a clean approach.

Comment on lines +19 to +28
formatter.keyword('actions').append(Formatting.oneSpace());
const bracesOpen = formatter.keyword('{');
bracesOpen.prepend(Formatting.fit(Formatting.oneSpace(), Formatting.newLine()));
const bracesClose = formatter.keyword('}');
bracesClose.prepend(Formatting.newLine());
formatter.interior(bracesOpen, bracesClose).prepend(Formatting.indent());

const stateName = formatter.property('name');
const stateEnd = formatter.keyword('end');
formatter.interior(stateName, stateEnd).prepend(Formatting.indent());
Copy link
Contributor Author

@Yokozuna59 Yokozuna59 Nov 4, 2023

Choose a reason for hiding this comment

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

One more thing, how to double indent? Currently, the following code:

state ExampleState
    actions {
        commandOne
    }
end

Would be formatted to:

state ExampleState
    actions {
    commandOne
    }
end

I tried to do the following:

formatter.interior(bracesOpen, bracesClose)
    .prepend(Formatting.indent())
    .prepend(Formatting.indent());

But it didn't work.

Copy link
Member

Choose a reason for hiding this comment

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

I took a look at this, and this seems to be an issue in the formatter. This is probably something we want to fix in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, I think the outer indent (the one for state and end) overrides this one.
Or maybe it doesn't take the double indent in consideration.

@msujew msujew requested a review from pluralia November 6, 2023 22:30
@Yokozuna59 Yokozuna59 requested a review from pluralia April 24, 2024 06:20
@Yokozuna59
Copy link
Contributor Author

Sorry for the wait, but I have applied the comments in this PR.

Although I don't know how to make the ECA test pass, I have created an account there, but previous commits are failing.

@Yokozuna59 Yokozuna59 marked this pull request as ready for review April 24, 2024 07:40
@Yokozuna59 Yokozuna59 force-pushed the yokozuna/add-more-services-to-statemachine branch from c94d708 to a990392 Compare April 24, 2024 12:26
@msujew
Copy link
Member

msujew commented Apr 29, 2024

@Yokozuna59 Can you squash your commits? I believe the ECA tool has some weird issue with some of your commits. Maybe you might also need to remove the verification requirement on your commits? All of them (except for the merge commit) say that they are Unverified.

@Yokozuna59

This comment was marked as resolved.

@Yokozuna59 Yokozuna59 force-pushed the yokozuna/add-more-services-to-statemachine branch 3 times, most recently from 4940b78 to b1a642d Compare April 29, 2024 14:02
- add `Formatting` service
- add `CodeActionProvider` service
    - Remove an unreachable commands, events, and states
    - Rename all refereed state names that don't start with caps
- validate unreached commands, events, and states
@Yokozuna59 Yokozuna59 force-pushed the yokozuna/add-more-services-to-statemachine branch from b1a642d to e3757c3 Compare April 29, 2024 14:08
@Yokozuna59
Copy link
Contributor Author

@msujew I squashed the commits and removed the commit verification, but the ECA is still failing.

@spoenemann
Copy link
Contributor

@Yokozuna59 have you used exactly the same email address in your Eclipse account as in the Git commit?

@Yokozuna59
Copy link
Contributor Author

@spoenemann Yes. Before rebasing and after creating the account, it showed that commits after that passed the test, but the original commits before creating the account still failed.

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.

Add features for the Statemachine example
4 participants