neo4j-contrib / rabbithole

Interactive, embeddable Neo4j-Console
http://rabbithole.heroku.com
79 stars 41 forks source link

Inline comments get parsed incorrectly, causing actions to happen! #55

Open aseemk opened 11 years ago

aseemk commented 11 years ago

Fittingly enough, I can't link to a test case, because the "share" functionality suffers from the same bug.

Manual repro:

1) Go to http://console.neo4j.org/.

2) Edit the query to add // delete r on a line before the return.

3) Notice that the parsed query has the // at the end of the previous line, and the delete is uncommented. (The return now creates an error, but if you remove the return, the destructive deletes will happen.)

In general though, why do any custom processing of the text at all? (E.g. squashing the multi-line to a single-line in the share box.)

While I like the console a lot, I dislike this greatly, as it always (a) strips blank lines, (b) adds trailing whitespace, and (c) just generally leaves me worrying whether my query will get broken, as in this case.

Please strongly consider just removing any custom processing, which will fix this bug too. =) Thanks!

freeeve commented 11 years ago

The autoformat/syntax highlighter can be greatly improved. I wrote the current version in like an hour, without any knowledge of how to do it, so it was just hacked together. If you or anyone here wants to take a shot at it (you do a lot of JS, right? :P), it's right here (and yes, I know it's kind of horrible, even though I still think it's much better than a single line input): https://github.com/neo4j-contrib/rabbithole/blob/master/src/main/webapp/javascripts/cypher.js#L152

Here are some examples of how to implement a syntax highlighter/autoformatter: http://codemirror.net/demo/formatting.html https://github.com/marijnh/CodeMirror/blob/master/mode/sql/sql.js https://github.com/marijnh/CodeMirror/blob/master/mode/clike/clike.js

The autoformat part could be stripped out (just take out that last function). At the time, I thought the backend needed single line strings to be sent to the server (so I would break everything to a single line and send, and then autoformat again--it no longer breaks it to a single line), but that isn't actually the case. The main benefit autoformatting provides is that you can type an ugly single line query, and it formats it for you automatically when it sends it back. In particular, when you share a cypher query, it doesn't save linebreaks (AFAIK), so the only way to have them is with the autoformatting.

I will probably get to look at it again eventually, but it's low on my list. I'm currently having fun writing neostore code in C. :)

jexp commented 11 years ago

At least the console respects newlines in the server, so we are save to send them.

I'm not sure about the automatic formatter, I actually like it to take care of my query. Sometimes it messes up, but I think adding comments shouldn't be that hard for a JS crack.