Skip to content

Handle multiline command-line args #363

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Handle multiline command-line args #363

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions mprof.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import glob
import os
import os.path as osp
import shlex
import sys
import re
import copy
Expand Down Expand Up @@ -175,8 +176,7 @@ def clean_action():

def get_cmd_line(args):
"""Given a set or arguments, compute command-line."""
blanks = set(' \t')
args = [s if blanks.isdisjoint(s) else "'" + s + "'" for s in args]
args = [shlex.quote(s).replace('\n', '\\n') for s in args]
Copy link

Choose a reason for hiding this comment

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

I don't think using shlex.quote is the right choice here, because it quotes strings in ways that are parseable by the shell, and the strings encoded in the file will never be parsed by the shell. But the shell-escaped versions look ugly in the plots.

For example, this command:

mprof echo "Hello's fine"

...will be escaped by the master branch code as echo 'Hello's fine' which looks sort of nonsensical but it's fine, it doesn't matter if the quoting is legal, just that it's intelligible in the plot.

Using shlex.quote, though, what you get for a plot label is not really an improvement:

image

Doing the .replace('\n', '\\n') is a good idea (though I think newlines could also just be replaced with a space, and it would be fine), but the quoting should be left to simple, not-technically-correct-but-we-get-the-idea logic like in the previous code.

return ' '.join(args)

def find_first_process(name):
Expand Down