Skip to content

Do not complete paths in attr unless beginning of value looks like a path #1293

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 4, 2022
Merged

Do not complete paths in attr unless beginning of value looks like a path #1293

merged 1 commit into from
Oct 4, 2022

Conversation

@datho7561
Copy link
Contributor

@datho7561 datho7561 commented Sep 14, 2022

Fixes redhat-developer/vscode-xml#668

Signed-off-by: David Thompson [email protected]

@datho7561 datho7561 force-pushed the 668-fix-path-autocompletion branch 2 times, most recently from 367641e to e85f353 Compare September 15, 2022 14:12
@datho7561 datho7561 force-pushed the 668-fix-path-autocompletion branch 2 times, most recently from 1a65d09 to b30db51 Compare September 15, 2022 19:55
@datho7561
Copy link
Contributor Author

datho7561 commented Sep 22, 2022

@angelozerr if you have time, could you please take a look at this?

@angelozerr
Copy link
Contributor

angelozerr commented Sep 22, 2022

I promise you to review all your PR as soon as I will finish my contribution with generic support for qute.

rgrunber
rgrunber approved these changes Sep 22, 2022
View reviewed changes
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Change looks pretty good to me so I think we can probably merge. Just a minor adjustment.

@datho7561 datho7561 force-pushed the 668-fix-path-autocompletion branch from b30db51 to 243006d Compare September 22, 2022 19:34
@datho7561
Copy link
Contributor Author

datho7561 commented Sep 22, 2022

I think it'd be good to check that this works properly on Windows before merging. I haven't been able to test if it works there yet.

@angelozerr
Copy link
Contributor

angelozerr commented Sep 28, 2022

@datho7561 could you move the file path completion participant and the test in extensions/filepath instead of extensions/general.

@angelozerr
Copy link
Contributor

angelozerr commented Sep 28, 2022 •
edited
Loading

I'm sorry @datho7561 several tests on Windows OS fail.

  • testFilePathCompletionForEmptyDoctype
  • testFilePathCompletionFolderBAbsolutePathBackSlash
  • testFilePathCompletionFolderBAbsolutePath

It found 0 items for those tests.

I need to find time to investigate the problems.

@datho7561
Copy link
Contributor Author

datho7561 commented Sep 28, 2022

I can investigate it on my Windows machine. I'll try to get these fixed today

@datho7561 datho7561 force-pushed the 668-fix-path-autocompletion branch from 243006d to 8f39691 Compare September 28, 2022 13:22
@datho7561
Copy link
Contributor Author

datho7561 commented Sep 28, 2022

Let me know if this works for you, Angelo. I think something is wrong with CodeQL, and Jenkins didn't trigger for some reason, so I'll check the changes under linux

@datho7561 datho7561 force-pushed the 668-fix-path-autocompletion branch from 8f39691 to 889b90e Compare September 28, 2022 14:13
@datho7561
Copy link
Contributor Author

datho7561 commented Sep 28, 2022

@angelozerr could you check now please?

@datho7561
Copy link
Contributor Author

datho7561 commented Oct 4, 2022

@angelozerr is it okay to merge this PR?

@angelozerr angelozerr merged commit 4f794cf into eclipse-lemminx:main Oct 4, 2022
@angelozerr
Copy link
Contributor

angelozerr commented Oct 4, 2022

Tests pass now in Windows OS. Thanks @datho7561 !

@angelozerr angelozerr added this to the 0.22.0 milestone Oct 4, 2022
@datho7561 datho7561 deleted the 668-fix-path-autocompletion branch October 4, 2022 20:22
@angelozerr angelozerr added the enhancement New feature or request label Oct 17, 2022
@angelozerr angelozerr mentioned this pull request Oct 24, 2022
Closed
@fbricon fbricon mentioned this pull request Nov 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@angelozerr angelozerr Awaiting requested review from angelozerr

1 more reviewer

@rgrunber rgrunber rgrunber approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

0.22.0

Development

Successfully merging this pull request may close these issues.

Unwanted autocompletion

3 participants

@datho7561 @angelozerr @rgrunber