rethinkdb / rethinkdb

The open-source database for the realtime web.
https://rethinkdb.com
Other
26.74k stars 1.86k forks source link

Data explorer doesn't wrap properly #5873

Open mlucy opened 8 years ago

mlucy commented 8 years ago

In Chrome on Windows, if I type enough in the data explorer that the line wraps, the visual location of the cursor becomes disconnected from where the cursor actually is. In my case the actual location of the cursor was two characters earlier, so if I position the cursor at the end of the word fold and type ., the buffer containsfo.ld.

Here's the exact snippet that was triggering it for me:

r.expr([1, 5, 8, 10]).union([2, 7, 8], [2, 2, 9], {interleave: function(x) { return x; }}).map(function(x) { return {id: x}).fold(nil, function(acc, obj) { return obj('id'); }, emit: function(acc, obj

This is actually extremely annoying because it makes the data explorer very difficult to use for large queries. (If I hard-wrap the line the problem goes away, but that isn't always easy to do.)

marshall007 commented 8 years ago

I've been running into this too. Would be great if we could have a Data Explorer setting for disabling wrapping entirely.

danielmewes commented 8 years ago

Pinging @deontologician

deontologician commented 8 years ago

I thought it might have been codemirror/CodeMirror#867, but it looks like that was fixed in 3.0, and according to this line the version of CodeMirror we have should have fixed it (3.11).

It does seem like we can just disable lineWrapping here, so that's an option.

The other option is to upgrade Codemirror (seems like our version is ancient) and it might fix this. Though I don't know how much custom hacking we did on it for typeahead etc. This seems riskier.

deontologician commented 8 years ago

I guess a third option is to go hacking around in the codemirror source and fix the bug for real, that seems like a time sink

mglukhovsky commented 8 years ago

I ran into this on Chrome on OS X, assuming this is the same issue:

bug

This is the query that generated it:

r.db('photobooth').table('photos').replace((p) => p.merge({filename: r.expr('/home/michael/thinkerbooth/').add(p('filename'))}))
chrisvariety commented 8 years ago

(I've been seeing this issue for a while but assumed it was due to my out-of-date RethinkDB)

https://github.com/codemirror/CodeMirror/issues/1913 https://github.com/codemirror/CodeMirror/issues/1957

Looks like it should be fixed in v4 of CodeMirror, based on the comments in those issues I'm not sure you really have choices other than upgrading CodeMirror to at least v4 -- there's no easy patch. (Disabling line wrapping would be awful)

deontologician commented 8 years ago

This is tricky. So it does seem upgrading codemirror is probably the best way forward (and something we should probably do eventually anyway). We don't have any regression tests for the dataexplorer though. Maybe this would be a good time to add some...

danielmewes commented 8 years ago

Is there a good way for automatically testing the Data Explorer? Or should we just compile some test cases for manual testing?

deontologician commented 8 years ago

Parts of it could be automatically tested, there aren't even unit tests.

Looking into it, it seems there's been a few custom modifications to the codemirror source: https://github.com/rethinkdb/rethinkdb/commits/next/admin/static/js/codemirror/codemirror.js

This'll probably be a "try it and see" kind of job

neumino commented 8 years ago

FWIW, I didn't tweak the internal of code mirrors, just set up a few modules. Upgrading should be relatively safe (I remember doing it without any hiccup at some point).

ghost commented 8 years ago

Any progress on this issue? I also have the same problem and it's really annoying

OriginalEXE commented 8 years ago

Was just going to open an issue for this, thank you for looking into it.

I found another issue that will also probably be fixed with the library upgrade, but let me know if I should open a separate issue to track it.

Basically, when you use the arrow function in the data explorer, the indentation comes out wrong. Here's an example of what I am talking about:

rethinkdb administration console

frankleng commented 8 years ago

seems to be fine in firefox

xfg commented 8 years ago

+

danielmewes commented 8 years ago

Let's definitely fix this in RethinkDB 2.4.

sagivf commented 7 years ago

Hello hello. 1) I've started working on this issue. 2) It seems like Codemirror 5.23.0 fixes the issue. 3) I'll test locally for a few days and then add pull requests here and in https://github.com/atnnn/rethinkdb-webui. 4) I'll open a new task on moving this dependancy to npm. I think committing the source code is not ideal.

sagivf commented 7 years ago

Updating to 5.23.0 seems to be working nicely in chrome. But I think I was over optimistic. Using the keyboard works without the bug. But the bug still occurs when placing the cursor with the mouse.

1) Has anyone used Codemirror and have any idea where to look? 2) Seems like safari and firefox are buggy with the keyboard and autocomplete. What browsers are we officially supporting?

Here is the brach: https://github.com/AtnNn/rethinkdb-webui/tree/fix-data-explorer-wrap-5873 (don't forget to build)

I'll keep digging but any advice is welcome...

chrisvariety commented 7 years ago

This bug seems to have fixed itself on Chrome Version 59.0.3071.86 on Mac. Can anyone else confirm?

sagivf commented 7 years ago

@chrisvariety I have chrome 58.0.3029.110 on Mac and it seems to work 😄 This is great as It doesn't seem like an easy fix.

chrisvariety commented 7 years ago

Upon further reflection, I am still seeing occasional issues, but it does seem better.

trickkiste commented 7 years ago

I am still having this problem with latest Firefox on Mac OSX.

GeoffreyPlitt commented 7 years ago

+1 Chrome 60 on Mac 10.12.6, ReThinkDB=2.3.6