Skip to content

Conversation

@stepan-anokhin
Copy link

@stepan-anokhin stepan-anokhin commented Sep 17, 2022

Fix the problem from #210

The problem occurs when mprof run <command> has multi-line arguments:

For example:

read -r -d '' MESSAGE << EOM            
  hello
  world
EOM

mprof run echo "$MESSAGE"
mprof plot
Using last profile data.
Traceback (most recent call last):
  File "/home/stepan/mprof-example/venv/bin/mprof", line 8, in <module>
    sys.exit(main())
  File "/home/stepan/mprof-example/venv/lib/python3.8/site-packages/mprof.py", line 905, in main
    actions[get_action()]()
  File "/home/stepan/mprof-example/venv/lib/python3.8/site-packages/mprof.py", line 819, in plot_action
    mprofile = plotter(filename, index=n, timestamps=timestamps, options=args)
  File "/home/stepan/mprof-example/venv/lib/python3.8/site-packages/mprof.py", line 409, in plot_file
    mprofile = read_mprofile_file(filename)
  File "/home/stepan/mprof-example/venv/lib/python3.8/site-packages/mprof.py", line 364, in read_mprofile_file
    field, value = l.split(' ', 1)
ValueError: not enough values to unpack (expected 2, got 1)

"""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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants