8000 Using "\n" in an attribute value gets escaped to "\\n" on save · Issue #1739 · TrenchBroom/TrenchBroom · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

Using "\n" in an attribute value gets escaped to "\\n" on save #1739

Closed
ericwa opened this issue Mar 18, 2017 · 2 comments
Closed

Using "\n" in an attribute value gets escaped to "\\n" on save #1739

ericwa opened this issue Mar 18, 2017 · 2 comments
Assignees
@ericwa
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Bug Errors and problems
Milestone
TrenchBroom 2.0.0

Comments

@ericwa
Copy link
Collaborator
ericwa commented Mar 18, 2017

RC3.
The recent changes to escaping broke the basic use case of typing a message with newlines in it.
i.e. Create a trigger_once in TB, and type in a message key of "First line\nsecond line". Save.
It should appear in the map file as "message" "First line\nsecond line" but instead it's "message" "First line\\nsecond line"
@kduske kduske added this to the TrenchBroom 2.0.0 milestone Mar 19, 2017
@kduske kduske added Platform:All Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Bug Errors and problems labels Mar 19, 2017
@ericwa
Copy link
Collaborator Author
ericwa commented Mar 19, 2017

Aside from this case, my Quake compile tools recognize a \b escape sequence (which toggles orange text) and I'm expecting that mappers can type that in messages. At the same time, it shouldn't be something TB needs to know about / have support for.

Maybe it was a mistake to do automatic unescaping/escaping between the .map file and the text edit box in TB?

What about this:

  • support reading maps with \" occurring in key/values
  • if the user enters an un-escaped " character in TB's key/value editor, show an error or immediately convert it to \"
  • otherwise, present the keys/values to the user as-is, don't do any transformation between the string in the .map file and what's presented in the key/value editor

@ericwa ericwa self-assigned this Apr 8, 2017
ericwa added a commit that referenced this issue Apr 8, 2017
@ericwa
#1739: Add failing tests
ab8ae6e
@ericwa ericwa mentioned this issue Apr 8, 2017
Merged
kduske pushed a commit that referenced this issue Apr 21, 2017
@ericwa @kduske
#1739: don't escape/unescape entity keys/values, except escaping " to…
a422dca 
… \" (#1759)

* #1739: Add failing tests

* NodeSerializer::escapeEntityAttribute: only escape bare " characters, nothing else

* NodeWriterTest: add writePropertiesWithEscapedQuotationMarks

* Change WorldReaderTest to no longer expect strings to be unescaped

* StandardMapParser::parseEntityAttribute(): Don't unescape

* 1739: factor out escaping code to StringUtils::escapeIfNecessary

* Remove obsolete comment
@kduske kduske closed this as completed Apr 21, 2017
@parasti
Copy link
parasti commented May 4, 2017

Just to add some data to this, interpreting \" as an escape sequence is not entirely historically sound, since it does break certain maps that have been made with the Radiant lineage of editors. In particular, when the backslash character is the last character in a value, for example:

{
  "classname" "worldspawn"
  "message" "Some game\that questionably uses\backslash for\newlines.\"
}

{
  "classname" "path_corner"
}

This may not be particularly smart in retrospect, but historically it has worked (on most editors). Neverball has been getting away with this since 2003. https://github.com/Neverball/neverball/blob/3bec493e439922676308f928ee3807a75c0f450c/data/map/01_easy.map

If anything needs to be done, I can't really say, as I'm not familiar with your goals. Just dropping a bit of data for your consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ericwa ericwa

Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Bug Errors and problems
Projects
None yet
Milestone
TrenchBroom 2.0.0
Development

No branches or pull requests
3 participants
@parasti @kduske @ericwa
0