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

Editing string variable value removes newline characters '\n' in variables debug view #73975

Closed
jeanp413 opened this issue May 20, 2019 · 10 comments · Fixed by #74144
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues *out-of-scope Posted issue is not in scope of VS Code
Milestone

Comments

@jeanp413
Copy link
Contributor

Steps to Reproduce:

  1. Start debugging and try to edit a string variable value containing newline characters '\n' in variables debug view.
  2. Press enter (without modifying variable value), observe the value have been modified and the newline characters '\n' have been removed.
    bug-variablesview
@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label May 21, 2019
@NizamLZ
Copy link
Contributor

NizamLZ commented May 22, 2019

Looking into it

@isidorn isidorn added the bug Issue identified by VS Code Team member as probable bug label May 23, 2019
@isidorn isidorn added this to the May 2019 milestone May 23, 2019
@isidorn
Copy link
Contributor

isidorn commented May 23, 2019

@jeanp413 we have merged a PR which tackles this issue. Please try it out in tomorrows vscode insiders and let us know if the issue is fixed for you. Thank you

@jeanp413
Copy link
Contributor Author

@isidorn @NizamLZ I tested it but it generates the wrong value sometimes
testcase

I did some research and it seems we also need to unscape the string when we get the new value from the input

export function replaceWhitespace(value: string): string {
	const map: { [x: string]: string } = { '\n': '\\n', '\r': '\\r', '\t': '\\t', '\b': '\\b', '\v': '\\v', '\f': '\\f' };
	return value.replace(/[\n\r\t\b\v\f]/g, char => map[char]);
}
function replaceEscapedWhitespace(value: string): string {
	const map: { [x: string]: string } = { '\\n': '\n', '\\r': '\r', '\\t': '\t', '\\b': '\b', '\\v': '\v', '\\f': '\f' };
	return value.replace(/\\[nrtbvf]/g, escapedWhitespace => map[escapedWhitespace]);
}

options.onFinish(inputBox.value, renamed);

change this line ⬆️ to

options.onFinish(replaceEscapedWhitespace(inputBox.value), renamed);

@NizamLZ
Copy link
Contributor

NizamLZ commented May 25, 2019

@jeanp413 Thanks for testing. You want to submit a PR or should I?

@jeanp413
Copy link
Contributor Author

@NizamLZ go ahead 👍

@jeanp413
Copy link
Contributor Author

jeanp413 commented May 26, 2019

@isidorn @NizamLZ I did some more testing using the python extension and with both changes (the original PR and my suggestion) it breaks because the python extension also unescapes the newlines.
So I think we need to agree where the unescaping is done if inside vscode or the extensions.
This is somewhat related to #73845

@isidorn
Copy link
Contributor

isidorn commented May 28, 2019

In theory VSCode should not modify the original values.
We only change them for presentation purposes.
@jeanp413 would you like to provide a PR which cleans this up?
And easy starting point would be just looking where the replaceWhitespace function is being used. And it should only be used to make something easier to read, not to change the actual value.

@jeanp413
Copy link
Contributor Author

@isidorn I looked at the places where replaceWhitespace is being used and I think those are OK. After reading the Debug Adapter Protocol have a few observations:

  • Let's say an additional input widget, for editing a variable's value, exists (could be the case that another editor use this widget) that uses a textarea (instead of a textinput) then, for this widget, using escaped sequences is not needed while editing the value of a string variable. The new value obtained from this widget is send to the debugging extension and should be used as is (must not unescape the newlines) to update the variable value. In contrast using a textinput, we need to escape newlines and when sending the new value the extension should unescape them. But currently there's no way for an extension to know when to unescaped or not a string. (if this is the case, this should be handled in the editor or update the DAP with an additional parameter in the request)
  • Or strings values returned from the extension must always have newlines, etc, escaped and while editing them we always need to escape newlines,etc. (if this is the case then I'll open an issue in the node extension and I think replaceWhitespace would no longer be needed)

@Tyriar
Copy link
Member

Tyriar commented May 29, 2019

It seems like the initial state of test3 on the variable pane should be "\\n" based on how it's printed into the output. Going to reopen as the general issue doesn't seem to be fixed (editing a variable without making a change, changes it).

@Tyriar Tyriar reopened this May 29, 2019
@isidorn isidorn modified the milestones: May 2019, June 2019 May 31, 2019
@isidorn isidorn modified the milestones: June 2019, Backlog Jun 20, 2019
@isidorn isidorn added the *out-of-scope Posted issue is not in scope of VS Code label Oct 9, 2019
@vscodebot
Copy link

vscodebot bot commented Oct 9, 2019

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that are not going to be addressed in the foreseeable future: We look at the number of votes the issue has received and the number of duplicate issues filed. More details here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!

@vscodebot vscodebot bot closed this as completed Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues *out-of-scope Posted issue is not in scope of VS Code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants