Skip to content

Escape labels when generating graph #557

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

Merged
merged 1 commit into from
Sep 5, 2020
Merged

Conversation

auxiliary
Copy link
Contributor

Profile comments can have various special symbols which are then inserted as labels when generating graphs. If symbols are not escaped, graph generation can fail. This PR fixes that.

@google-cla google-cla bot added the cla: yes label Sep 2, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.46%. Comparing base (1a94d86) to head (2926496).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #557      +/-   ##
==========================================
+ Coverage   68.44%   68.46%   +0.01%     
==========================================
  Files          78       78              
  Lines       16098    16108      +10     
==========================================
+ Hits        11018    11028      +10     
  Misses       4225     4225              
  Partials      855      855              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@auxiliary auxiliary force-pushed the master branch 3 times, most recently from db29f0d to a94a50b Compare September 3, 2020 05:48
// escapeForDot escapes double quotes and backslashes, and replaces Graphviz's
// "center" character (\n) with a left-justified character.
// See https://graphviz.org/doc/info/attrs.html#k:escString for more info.
func escapeForDot(s []string) []string {
Copy link
Collaborator

@aalexand aalexand Sep 4, 2020

Choose a reason for hiding this comment

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

Could this function construct and return a new slice? Its current signature suggests that it does that (because it returns a slice) but in fact it changes the input slice in place. The combination of in-place modification and returning a value is imo not good. Also, I can't say just from this local context if it's OK to modify caller's labels slice content, so I'd prefer not modifying its contents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, makes sense. Done.

@aalexand aalexand merged commit acf8798 into google:master Sep 5, 2020
giordano added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Nov 13, 2020
* Update pprof to latest revision

Bump from 20191205061153 => 20201109224723

My personal interest is to pull in google/pprof#564, which adds support for displaying names with `"` in them, which julia functions sometimes have (e.g. `var"#foo#23"`)

Includes:
- google/pprof#564
- google/pprof#575
- google/pprof#574
- google/pprof#571
- google/pprof#572
- google/pprof#570
- google/pprof#562
- google/pprof#561
- google/pprof#565
- google/pprof#560
- google/pprof#563
- google/pprof#557
- google/pprof#554
- google/pprof#552
- google/pprof#545
- google/pprof#549
- google/pprof#547
- google/pprof#541
- google/pprof#534
- google/pprof#542
- google/pprof#535
- google/pprof#531
- google/pprof#530
- google/pprof#528
- google/pprof#522
- google/pprof#525
- google/pprof#527
- google/pprof#519
- google/pprof#520
- google/pprof#517
- google/pprof#518
- google/pprof#514
- google/pprof#513
- google/pprof#510
- google/pprof#508
- google/pprof#506
- google/pprof#509
- google/pprof#504

* Update P/pprof/build_tarballs.jl - use a real version number

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Remove now unused `timestamp`

* [pprof] Use `GitSource`

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
gmarin13 pushed a commit to gmarin13/pprof that referenced this pull request Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants