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

fix/suggestion item title #659

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

p-spacek
Copy link
Contributor

@p-spacek p-spacek commented Feb 11, 2022

What does this PR do?

  • The custom title of the suggestion item didn't work properly.
  • Add a description of the schema in 'object/parent' suggestion description.

This PR fix logic of the schema.title.

I had a problem, that the previous title could be modified by title from parents. So schema.title could contain a title from its parent. And then I wasn't able to return the correct name of a suggestion item (display of the real title, name of the ref object, parent title).

So this PR adds a new closestTitle prop, that should cover previous functionality.

I went through all schema.title and tried to decide where to use title or closesTitle. Results of the UT are ok, but I will be better to check this from your side.

What issues does this PR fix or reference?

no ref

Is it tested? How?

Modified one UT that covers this change

@@ -81,7 +81,8 @@
"build:libs": "yarn compile:umd && yarn compile:esm",
"compile:umd": "tsc -p ./tsconfig.umd.json",
"compile:esm": "tsc -p ./tsconfig.esm.json",
"check-dependencies": "node ./scripts/check-dependencies.js"
"check-dependencies": "node ./scripts/check-dependencies.js",
"pull-remote": "git pull https://github.com/redhat-developer/yaml-language-server.git main"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change shouldn't be there.
I can remove this if you don't want this in scrips...

but this command is useful to merge changes from your repo into forked one...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don’t think that we will use that script, but I'm OK to keep it

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to remove. It will confuse me :)

@coveralls
Copy link

coveralls commented Feb 11, 2022

Coverage Status

Coverage increased (+0.04%) to 81.186% when pulling bf11563 on p-spacek:fix/suggestion-item-title into 2f34acf 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.

Thank you

@evidolob evidolob merged commit 77e2f6c into redhat-developer:main Feb 15, 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

5 participants