federomero / pretty-json

Atom plugin. Format JSON documents.
MIT License
94 stars 23 forks source link

Prettify tries to parse other text when only json is selected #40

Closed ryan-durkin closed 8 years ago

ryan-durkin commented 8 years ago

To Reproduce: Create new empty file Put some text at the top ("Hello This is some text") Put some json after this line { "blah": "blee", "arr": [] }

Execute the Prettify Json command with the json text selected.

Result: JSON Pretty: SyntaxError: Unexpected 'H' at character 1 near "Hello This is some other text

Expected: JSON Pretty should only try to "prettify" the selected text

lexicalunit commented 8 years ago

Ah, I see why this is happening. There are two cases when pretty-json will attempt to format the entire file:

  1. The file's grammar is set to source.json.
  2. The file's grammar is the default null grammar when a new file is opened.

I think a better solution would be to only attempt to format the entire if if one of the above cases is true and the user has not selected any text in the file. If the user has selected text, we should probably always choose to format only the selected text. That would follow the principle of least surprise imho.

@ryan-durkin thank you for the report! Working on resolving this now.

lexicalunit commented 8 years ago

Ok, just released version 1.3.0. Let me know if this doesn't resolve this issue for you, @ryan-durkin.

ryan-durkin commented 8 years ago

Fixes the problem, @lexicalunit .

One other oddity, but it's not that big of a deal, is that after the json is formatted, the text that is highlighted is still the original line(s) rather than highlighting the newly formatted json.

For example, after formatting a single minified line of json into 5 lines of formatted json, only the first line is highlighted. I would expect my highlight to be on all the 5 lines. Again not a big deal =).

Thanks for the awesome turnaround.

lexicalunit commented 8 years ago

Oh interesting. That would be a nice thing to work properly. I didn't even notice it myself. Thanks, I'll cut a new issue :)