-
Notifications
You must be signed in to change notification settings - Fork 609
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
Escape Nodelet/tag names for Dot as well #568
Open
NHDaly
wants to merge
10
commits into
google:main
Choose a base branch
from
NHDaly:nhd-dotgraph-escape-labels--escape-Tags
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Escape Nodelet/tag names for Dot as well #568
NHDaly
wants to merge
10
commits into
google:main
from
NHDaly:nhd-dotgraph-escape-labels--escape-Tags
+89
−14
Commits on Oct 3, 2020
-
Add missing
escapeForDot()
to labels for function namesIn some programming languages, e.g. JuliaLang, function names can contain arbitrary characters. These are represented via the string macro `var"..."`, which allows constructing identifiers that wouldn't otherwise parse. These names are handled correctly by `pprof` in the FlameGraph view, but before this commit, they would produce an invalid dot file. This fixes the dot graph export for names that contain `"`.
Configuration menu - View commit details
-
Copy full SHA for 0b75a10 - Browse repository at this point
Copy the full SHA 0b75a10View commit details -
Configuration menu - View commit details
-
Copy full SHA for 866a19c - Browse repository at this point
Copy the full SHA 866a19cView commit details -
Apply
escapeStringForDot()
in more places, to cover more cases of p……otentially harmful string labels. Remove mistaken `escapeStringForDot()` around tag names
Configuration menu - View commit details
-
Copy full SHA for 5d0860e - Browse repository at this point
Copy the full SHA 5d0860eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 04c65fd - Browse repository at this point
Copy the full SHA 04c65fdView commit details -
Configuration menu - View commit details
-
Copy full SHA for 72389be - Browse repository at this point
Copy the full SHA 72389beView commit details
Commits on Oct 5, 2020
-
Configuration menu - View commit details
-
Copy full SHA for dff1afc - Browse repository at this point
Copy the full SHA dff1afcView commit details -
Configuration menu - View commit details
-
Copy full SHA for 956deb6 - Browse repository at this point
Copy the full SHA 956deb6View commit details -
Escape Nodelet/tag names for Dot as well.
This escaping requirement was previously leaking into the implementation of `graph.go`, by writing `\n` directly instead of "\n". This meant that if it was displayed in a different program besides `dot`, it would have been potentially incorrectly escaped. Now, this commit moves all the escaping directly into dotgraph, which is more encapsulated, and also allows us to escape other characters in the name which need escaping.
Configuration menu - View commit details
-
Copy full SHA for ce716bd - Browse repository at this point
Copy the full SHA ce716bdView commit details -
Add justify option to escapeForDot to preserve old behavior
This allows tag newlines to stay center justified, and legend label newlines to stay left justified, as they were before we started escaping them for Dot. Also ran gofmt
Configuration menu - View commit details
-
Copy full SHA for e739e7e - Browse repository at this point
Copy the full SHA e739e7eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 8c597aa - Browse repository at this point
Copy the full SHA 8c597aaView commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.