Ableton / maxdevtools

MIT License
271 stars 14 forks source link

maxdiff should be robust to malformed json files #24

Closed diemoschwarz closed 4 months ago

diemoschwarz commented 7 months ago

It happens that git produces merge conflict marks in patches. This causes maxdiff to bail. It should probably default to raw file output, maybe with a warning inserted at the top? Or can the json parsing be recovered, just ignoring the broken dicts?

diffbad.maxpat.zip

/sw/bin/python3 /Users/schwarz/src/maxdevtools-ableton/maxdiff/maxpat_textconv.py diffbad.maxpat

Traceback (most recent call last):
  File "/Users/schwarz/src/maxdevtools-ableton/maxdiff/maxpat_textconv.py", line 34, in <module>
    main()
  File "/Users/schwarz/src/maxdevtools-ableton/maxdiff/maxpat_textconv.py", line 26, in main
    result = parse(args.file)
  File "/Users/schwarz/src/maxdevtools-ableton/maxdiff/maxpat_textconv.py", line 13, in parse
    patcher_dict = json.load(file_obj)
  File "/sw/lib/python3.10/json/__init__.py", line 293, in load
    return loads(fp.read(),
  File "/sw/lib/python3.10/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/sw/lib/python3.10/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/sw/lib/python3.10/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting property name enclosed in double quotes: line 57 column 1 (char 1235)
cclauss commented 6 months ago

What happens when you python3 -m json.tool diffbad.maxpat ?

diemoschwarz commented 6 months ago
python3 -m json.tool diffbad.maxpat
Expecting property name enclosed in double quotes: line 57 column 1 (char 1235)
cclauss commented 6 months ago

Yes. This is an invalid file that the Python standard library json module that is unable to parse.

MattijsKneppers commented 5 months ago

@diemoschwarz I think it should be possible to show a warning in the diff if there are merge conflict markers in a file, instead of failing completely. Good point!

MattijsKneppers commented 4 months ago

Hi @diemoschwarz, I've gotten around to proposing a fix for this issue in this PR: https://github.com/Ableton/maxdevtools/pull/33

I wonder if you would have time to give it a look and let me know if this is what you would expect?