-
Notifications
You must be signed in to change notification settings - Fork 74
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
autoClosingBrackets are forced off if autoCloseTags is enabled #62
autoClosingBrackets are forced off if autoCloseTags is enabled #62
Conversation
a04213f
to
57581be
Compare
src/extension.ts
Outdated
@@ -75,6 +75,17 @@ export function activate(context: ExtensionContext) { | |||
|
|||
function getSettings(): JSON { | |||
let configXML = workspace.getConfiguration(); | |||
|
|||
let autoCloseTags = configXML.get("xml.completion.autoCloseTags"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should only happen on configuration change on the client side.
there's no need to send that config to the server
src/extension.ts
Outdated
|
||
let autoCloseTags = configXML.get("xml.completion.autoCloseTags"); | ||
let autoClosingBrackets = configXML.get("[xml]")["editor.autoClosingBrackets"]; | ||
if (autoCloseTags && autoClosingBrackets != "never") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to ask permission for changing the value. Not do it 1st and ask for forgiveness
b392503
to
6486e19
Compare
src/extension.ts
Outdated
(error) => console.log(error) | ||
); | ||
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be careful, this is the case when user presses escape. You don't want to do anything if action gets cancelled on purpose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, ill fix
6486e19
to
da64432
Compare
src/extension.ts
Outdated
let autoCloseTags = configXML.get("xml.completion.autoCloseTags"); | ||
let autoClosingBrackets = configXML.get("[xml]")["editor.autoClosingBrackets"]; | ||
if (autoCloseTags && autoClosingBrackets != "never") { | ||
window.showInformationMessage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showWarningMessage
da64432
to
fa23a10
Compare
6e221ab
to
ba94c4f
Compare
@fbricon Updated, I didnt add the preference for the warning though. It will just remember for the servers lifecycle and not ask again. |
ba94c4f
to
4e699ff
Compare
src/extension.ts
Outdated
let configXML = workspace.getConfiguration(); | ||
let autoCloseTags = configXML.get("xml.completion.autoCloseTags"); | ||
let autoClosingBrackets = configXML.get("[xml]"); | ||
console.log(autoClosingBrackets); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove log
src/extension.ts
Outdated
let autoCloseTags = configXML.get("xml.completion.autoCloseTags"); | ||
let autoClosingBrackets = configXML.get("[xml]"); | ||
console.log(autoClosingBrackets); | ||
autoClosingBrackets = autoClosingBrackets["editor.autoClosingBrackets"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a different variable name. this is too confusing
Few minor changes left to do but it looks pretty good now. |
Fixes redhat-developer#38 Signed-off-by: Nikolas Komonen <nikolaskomonen@gmail.com>
4e699ff
to
40859e1
Compare
@fbricon Updated |
No description provided.