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

change completion that is invoked just after array symbol #698

Conversation

p-spacek
Copy link
Contributor

@p-spacek p-spacek commented Apr 6, 2022

What does this PR do?

unify how codecompletion is displayed in this two situations:

  • - #curstor
  • -#curstor

current implementation:

Screen.Recording.2022-04-06.at.15.08.13.mov

after this update:

Screen.Recording.2022-04-06.at.15.10.56.mov

What issues does this PR fix or reference?

this PR extends:
#696
#697
but it's working separately

Is it tested? How?

modified existing unit tests
I also tested all 3 changes together. I didn't want to put it all together because it's quite different modifications.

@coveralls
Copy link

coveralls commented Apr 6, 2022

Coverage Status

Coverage increased (+0.1%) to 82.307% when pulling 668a1bc on p-spacek:fix/completion-just-after-array-simbol into 098496e on redhat-developer:main.

Copy link
Collaborator

@gorkem gorkem left a comment

Choose a reason for hiding this comment

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

One small comment, otherwise looks good.

@@ -71,6 +71,7 @@ export class YamlCompletion {
private configuredIndentation: string | undefined;
private yamlVersion: YamlVersion;
private indentation: string;
private prefixIndentation = '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this get a better name to indicate that this prefix is used for arrays only.

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 thought that this property can be used for very similar functionality in the future:
colon without space

simpleProp:#invoke here without space

but I can change the name to arrayPrefixIndentantion and when this functionality is implemented in the future we can change it back to something more generic.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless you are planning to provide a PR for it soon. I would prefer descriptive names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure when, so I renamed it.

@msivasubramaniaan msivasubramaniaan merged commit b3ba325 into redhat-developer:main Apr 11, 2022
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.

None yet

4 participants