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.

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

Fixes DTD completion #281

Merged
merged 1 commit into from
Jan 22, 2019
Merged

Fixes DTD completion #281

merged 1 commit into from
Jan 22, 2019

Conversation

NikolasKomonen
Copy link
Contributor

Fixes #232

@fbricon
Copy link
Contributor

fbricon commented Jan 17, 2019 •
edited
Loading

So according to the LSP spec :

$0 defines the final tab stop, it defaults to the end of the snippet

All snippets should be revisited

@NikolasKomonen
Copy link
Contributor Author

@fbricon Updated

@NikolasKomonen NikolasKomonen force-pushed the dtdCompletion branch 2 times, most recently from a996e2a to e4667cd Compare January 21, 2019 15:49
@NikolasKomonen
Copy link
Contributor Author

@fbricon updated snippets not supported case

fbricon
fbricon requested changes Jan 21, 2019
View reviewed changes
Copy link
Contributor

@fbricon fbricon left a comment

Choose a reason for hiding this comment

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

So it turns out using $0 in those completions is not a great idea. When you hit tab, the placeholder is replaced with tab, which, in the context of those specific completions doesn't work. That is, when you want to keep #PCDATA for instance, hitting tab wouldn't work as expected.

TL;DR: replace all $0 with the next placeholder value

@NikolasKomonen
Copy link
Contributor Author

@fbricon updated

fbricon
fbricon approved these changes Jan 21, 2019
View reviewed changes
Copy link
Contributor

@fbricon fbricon left a comment

Choose a reason for hiding this comment

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

ok to merge after https://github.com/angelozerr/lsp4xml/pull/281/files#r249522174 is addressed

@NikolasKomonen
Fixes DTD completion
3ab04d9 
Fixes eclipse-lemminx#232

Signed-off-by: Nikolas Komonen <[email protected]>
@NikolasKomonen NikolasKomonen merged commit 05895c4 into eclipse-lemminx:master Jan 22, 2019
@NikolasKomonen NikolasKomonen deleted the dtdCompletion branch January 22, 2019 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbricon fbricon fbricon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NikolasKomonen @fbricon