Add Hover + Go-to Definition Request Support by betterclever · Pull Request #717 · xi-editor/xi-editor

betterclever · 2018-06-19T23:59:36Z

Tracking Issues: #659 #693

Current Stage:
Hover: Working. On Option+Click in the Client. Markdown is not being parsed in Client yet.
Go-to definition: Work completed from Plugin Side. Needs attention from Client.

betterclever · 2018-06-21T18:02:09Z

@cmyr @nangtrongvuon You may have a look at this stage. I added an RPC for making Hover request on the client side. Currently, it may be mapped to a key-binding to trigger request at the current cursor location i.e. skipping position argument in RPC.

I would like if a separate branch in xi-mac with preliminary support for that can be made to facilitate further testing 😃

nangtrongvuon · 2018-06-24T04:50:31Z

You can test this on my branch hover-definition. I put in a request to call for a hover when you move the mouse.

betterclever · 2018-06-24T22:13:12Z

@nangtrongvuon I updated https://github.com/nangtrongvuon/xi-mac/blob/hover-definition/XiEditor/EditViewController.swift#L540 to match the RPC.

document.sendRpcAsync("request_hover_definition", params: ["request_id": 1, "position": ["type": "utf8_line_char", "line": hoverPosition.line, "character": hoverPosition.column]])

Now, I am also forwarding the hover result to client, see this diff

nangtrongvuon · 2018-06-26T15:30:42Z

Looks good, I'll work on that further!

cmyr

I've only got about halfway through review and I'm running out of gas, will pick this back up tomorrow. Here's the first half, though, in case it's useful. :)

cmyr · 2018-07-09T20:25:41Z

 use config::Table;
 use styles::ThemeSettings;
-use plugins::rpc::ClientPluginInfo;
+use plugins::rpc::{ClientPluginInfo};


Nit: we don't include the {braces} on single-item imports.

cmyr · 2018-07-09T20:26:55Z

    pub strings: Vec<String>,
 }

+#[derive(Serialize, Deserialize)]


Nit: public types should include at least some documentation.

cmyr · 2018-07-09T20:27:19Z

+    pub range: Option<Range>
+}
+
+/* impl From<PluginHoverResult> for HoverResult {


cmyr · 2018-07-09T20:27:55Z

    Resize(Size),
    RequestLines(LineRange),
+    RequestHover { request_id: usize, position: Option<Position> },
+    RequestDefinition { request_id: usize, position: Option<Position> }


Can we add these to the frontend docs?

cmyr · 2018-07-09T20:29:36Z

        view.invalidate_styles(&self.text, start, end_offset);
    }

+    pub fn get_text_rope(&self, rev: RevToken) -> Option<Cow<Rope>> {


Does this need to be public? In general I now prefer pub(crate) where possible, because it helps the compiler warn us when code becomes unused.

Also, maybe we could call this get_rev? We already have get_buffer, which returns the current head.

Also if we're adding this method (which seems good) we should also use it in plugin_get_data, below.

Yes. It needs to be public. It is being used in editor.rs . Applied other suggestions.

pub(crate) fn foo(..) is still useable from other files in the same crate.

Yes. I made this a pub (crate) function already.

Ah yea okay github isn't great at showing changes after review.

cmyr · 2018-07-09T20:42:24Z

        self.render_if_needed();
    }

+    pub fn get_client_hover_result(&mut self, rev: u64, result: HoverResult) -> ClientHoverResult {


Not sure this needs to be pub.

cmyr · 2018-07-09T20:48:07Z

    }
+
+    fn request_hover(&mut self, request_id: usize, position: Option<Position>) {
+


Nit: this line contains whitespace

cmyr · 2018-07-09T20:51:48Z

+
+    fn request_hover(&mut self, request_id: usize, position: Option<Position>) {
+
+        let position = position.or_else(|| self.view.borrow().get_caret_offset()


It's unclear to me that we need to send Position structs to the plugins; we currently expect all plugins to be handling utf-8 offsets, so we could just use that.

I could imagine at some point allowing plugins to specify the format that they receive positions in, but even in that case we wouldn't need to send the enum, we would only ever send one type to each plugin.

If you do want to keep it this way, these two functions can share a lot of code; maybe add a fn resolve_position(&self, position: Option<Position>) helper?

I am confused. If Plugins specify their preference i.e Utf16 Line/Char or Utf8 or anything else, how will that be handled without enum?

Currently, plugins only get sent utf8 offsets, and that isn't going to change soon.

If at some point in the future, we allow a plugin to specify it's preferred unit, the RPC it receives will still just be (probably) hover { position: { line: 0, col: 3 }, it will just know that col is in utf16 code units. We could represent this internally as an enum, (and that might be best) but we might not send it as an enum to the plugin; we could switch internally and modify the json as needed. (I'm not sure this is a good idea, just a thought).

cmyr · 2018-07-09T20:51:55Z

+    }
+
+    fn request_defintion(&mut self, request_id: usize, position: Option<Position>) {
+


more whitespace

cmyr · 2018-07-09T20:52:09Z

                           ed.is_pristine())
    }
+
+    fn request_hover(&mut self, request_id: usize, position: Option<Position>) {


mentioned above, we would call this do_request_hover.

cmyr · 2018-07-10T15:58:45Z

-                           max_size: usize,
-                           rev: RevToken) -> Option<GetDataResponse> {
-        let _t = trace_block("Editor::plugin_get_data", &["core"]);
+    pub (crate) fn get_rev(&self, rev: RevToken) -> Option<Cow<Rope>> {


Rust style has solidified on having no space between pub and (crate). I was also writing this like you are, but I'm changing to pub(crate) now.

cmyr · 2018-07-10T15:59:15Z

+                           max_size: usize,
+                           rev: RevToken) -> Option<GetDataResponse> {
+        let _t = trace_block("Editor::plugin_get_data", &["core"]);
+        let text_cow = match self.get_rev(rev) {


We can get fancy now and just do let text_cow = self.get_rev(rev)?;.

cmyr

Okay, I've gone through the rest of the core-lib changes. Still have to look at the lsp-lib stuff, hopefully later today.

I have some general concerns about the utf16 stuff. Specifically, I think that the best way of handling the utf8-utf16 conversions would be to add a new Metric to the Rope crate. I'd also like conversion routines to have unit tests, since it's an easy way to introduce hard-to-catch bugs.

I think this is probably worth doing right. I would suggest doing this in two parts, adding the Metric stuff and then fixing this up to use it. If we want to merge something temporary that would be possible, but I would add a FIXME for all the code we would be removing.

cmyr · 2018-07-10T16:59:48Z

    }
+
+    fn do_request_hover(&mut self, request_id: usize, position: Option<Position>) {
+        let position = position.or_else(|| self.view.borrow().get_caret_offset()


can we share more code between these two functions?

One thing that we might want to consider is having a helper on EventContext, with a signature like:

fn with_each_plugin<F: FnMut(&Plugin)>(&self, f: F) { .. }

cmyr · 2018-07-10T17:01:47Z

                                                "changes": changes}))
    }
+
+    pub fn get_hover(&self, view_id: ViewId, request_id: usize, position: &Position) {


Mentioned earlier, and just an observation: we may at some point want to have these two methods be RPC requests, that return results; that would let us more easily keep track of when all responses to a message had been received, in case we want to show that no hover or no definition is available.

(This is future work, no change needed now, but just something to think about).

cmyr · 2018-07-10T17:03:13Z

-    RemoveStatusItem { key: String }
+    RemoveStatusItem { key: String },
+    ShowHover { request_id: usize, result: Result<Hover, LanguageResponseError>, rev: u64 },
+    ShowDefinition { request_id: usize, result: Result<Definition, LanguageResponseError>, rev: u64}


Nit: I would add a trailing ,, so that the next time an item is added to this enum this item doesn't show up in the diff (like how RemoveStatusItem shows up in this diff, because we had to add the trailing ,).

cmyr · 2018-07-10T17:05:08Z

+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+#[serde(rename_all = "snake_case")]
+#[serde(tag = "type")]
+pub enum Position {


I would like some documentation for any public item in this file, because this file is currently the only real documentation available of the plugin protocol.

cmyr · 2018-07-10T17:06:03Z

+#[derive(Serialize, Deserialize, Debug, Clone)]
+#[serde(rename_all = "snake_case")]
+pub struct CompletionItem {
+    pub label: String,


Is this supposed to be part of this PR? It looks like it's part of the completions work.

In any case, I would also document what these various fields mean, and how they should be used.

cmyr · 2018-07-10T17:13:12Z

+
+#[derive(Serialize, Deserialize, Debug, Clone)]
+#[serde(rename_all = "snake_case")]
+pub struct TextEdit {


This is interesting. We of course have our own edit facilities, based on deltas and document revisions, so it's hard to know how meaningful this type is. We should at least be associating completion items with a rev. This also introduces a subtle problem with garbage collection of old revisions; I believe it would be possible for us to end up with completion items that are associated with a revision that no longer exists.

What we might want to do is convert this to a delta internally, and then transform that delta over its lifespan to keep it up to date with any new edits.

The idea was to receive such a TextEdit from Plugin and convert it to Delta in the Core, since it involves the same problem like converting Utf-16 Line/Char Range to Utf8 offsets in the Plugin, so I resorted to forwarding it to core.

cmyr · 2018-07-10T17:16:43Z

 //!
 //! [Serde]: https://serde.rs

+use internal::plugins::rpc::Position;


unnecessary internal

cmyr · 2018-07-10T17:17:39Z

    }

+
+    pub fn offset_to_line_col_utf16(&self, text: &Rope, offset: usize) -> (usize, usize) {


is this ever used?

No. One thing we might do is to send Utf16 L/C to Plugin directly after doing conversion from here though. But at this point it is not being used anywhere.

cmyr · 2018-07-10T20:06:11Z

@@ -0,0 +1,130 @@
+use lsp_types::*;


Needs a header / copyright notice

cmyr · 2018-07-10T20:07:01Z

+use types::DefinitionResult;
+use xi_plugin_lib::{Hover as CoreHover};
+use xi_plugin_lib::LanguageResponseError;
+use xi_plugin_lib::{


we can combine all of these imports I think?

cmyr

Okay, here's the last of my first pass here. As mentioned earlier I think we should do the utf8/utf16 conversion stuff in the rope crate; otherwise this is mainly little stuff.

cmyr · 2018-07-11T13:52:52Z

+    }
+
+    fn get_client_hover(&mut self, rev: u64, result: Hover) -> client::Hover {
+        return client::Hover {


I think the return is unnecessary here?

cmyr · 2018-07-11T13:53:50Z

    }

+    fn with_each_plugin<F: FnMut(&Plugin)>(&self, mut f:F) {
+        self.plugins.iter().for_each(|p| {


This can just be self.plugins.iter().for_each(f)

cmyr · 2018-07-11T13:56:39Z

+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
+#[serde(rename_all = "snake_case")]
+#[serde(tag = "type")]
+/// Position formats that can be sent/received from to Plugins


More nit stuff: we put the /// comments above the #[macro]s. Also, in general I would prefer to have a trailing , on the last item of each enum/struct, for the reasons mentioned earlier 😎

cmyr · 2018-07-11T14:11:25Z

        offset
    }

+    pub fn line_col_utf16_to_offset(&self, text: &Rope, line: usize, col: usize) -> usize {


I had a big comment here yesterday, I think, but it maybe got lost?

three things:

at an immediate level, this function is duplicating a ton of work that's done elsewhere. I think this could be made a lot simpler by just converting the utf16 col to a utf8, and then forwarding that to our line_col_to_offset fn.

at a higher level, these conversion utilities should be implemented in the rope crate. Specifically, converting between utf8 and utf16 should be done by a new Metric type.

In any case, these functions deserve unit tests. It's easy to have bugs here, and they're going to be annoying bugs to find later.

cmyr · 2018-07-11T14:12:41Z

+    let line_num = view.line_of_offset(offset)?;
+    let line_offset = view.offset_of_line(line_num)?;
+
+    let char_offset: usize = count_utf16(&(view.get_line(line_num)?[0..(offset - line_offset)]));


we shouldn't need this type annotation

cmyr · 2018-07-11T14:14:33Z

+    View,
+};
+
+pub fn marked_string_to_string(marked_string: &MarkedString) -> String {


All of these can probably be pub(crate).

cmyr · 2018-07-11T14:17:35Z

+
+    }
+
+    fn remove_status_item(&mut self, id: &String) {


In general, prefer &str to &String.

cmyr · 2018-07-11T14:20:38Z

+            "telemetry/event" => {
+
+            },
+            misc @ _ => match self.language_id.to_lowercase().as_ref() {


In general when working with these big match statements for RPCs, I find it's cleanest to just use the match arms to call some function that can perform the logic; otherwise you end up with 200+ line functions. 😎

cmyr · 2018-07-11T14:23:52Z


    fn config_changed(&mut self, _view: &mut View<Self::Cache>, _changes: &ConfigTable) {}
+
+    fn get_hover(


These three functions duplicate a lot of code, which makes me uneasy. I think we could probably refactor this more to reuse more code?

cmyr · 2018-07-11T14:24:17Z

+            Err(error) => {
+                eprintln!("Can't convert location to offset. Error {:?}", error);
+                let err = LanguageResponseError::PositionConversionError(format!("{:?}",error));
+                ls_client.core.display_hover_result(view_id, request_id, Err(err), rev);


is display_hover_result the correct fn here?

betterclever · 2018-07-12T13:25:18Z

I have done most of the changes suggested, however I am not sure about how to reduce duplicity of code in the lsp_plugin.rs

cmyr · 2018-07-12T17:23:25Z

@betterclever okay, I'll take a look and see if I can come up with some more concrete suggestions.

cmyr

Thanks for the documentation, it looks good.

I've added a few more little nits, as usual. 🤓

This is a big patch, and I'm struggling to keep it all in my head at once. At a high level, two things jump out at me as not feeling totally consistent: the position conversions, and the error handling.

Unfortunately, i'm having trouble being much more precise in my criticism than, "I have a funny feeling".

Conversions

to recap: internally, xi generally represents positions as utf8 offsets. In communicating with the frontend, sometimes xi will receive positions as a line/col pair, where col is a utf8 offset; in this case we generally convert to a pure offset as early as possible.

The language server protocol uses utf16 line/col for most things. IIRC, we decided to handle conversion between utf16 and utf8 internally, because core is better suited to handle this logic.

This means that some plugins will, for some RPCs, be sent line/cols in utf16 space.

Q: How do we decide what plugins or what methods are sent utf-16?

receiving line/cols in different formats isn't the end of the world, since we can just pass them through some normalizer. Sending them is trickier, though, since we need to know for each recipient what we should be sending.

In any case, we would like to ensure that this conversion logic happens in one place, one time. If a plugin is going to handle utf-16, it should be converted in core. If a plugin is going to send utf-16, it will be converted in core. We do not want to duplicate this conversion logic between core and the plugin-lib; ideally what we want is some single method that Position and related types get passed to right before and after the RPC boundary, that performs the necessary conversion.

one option for this (at least for core->plugin methods) would be to always send line/col pairs in both utf-8 and utf-16. This would save us from having to do any fancy conversion in core, and would leave the plugin free to pick whichever encoding suited it best.

What about the client?

We added utf16 support for the language server stuff because we don't control all language server implementations, and language servers are using utf16 already.

We do get to dictate things to frontends, though, and to date we have told them to work with utf-8. This is minorly inconvenient, sometimes, because utf-16 is used in lots of gui toolkits, but conversion is relatively easy.

Using utf-16 with plugins shouldn't (directly) imply that we use it with clients as well. It is unclear to me that supporting utf-16 as an equal peer to utf-8 is worth the added complexity. I'm open to having my mind changed, but right now this seems like too much hassle.

Error handling

Something else in this PR that seems slightly inconsistent is the error handling story. I encouraged you to add some error handling in an earlier revision, and I appreciate that you've moved in that direction; however the error handling right now still feels somewhat bolted on.

There's a PositionConversionError, but we only seem to use this when the plugin-lib has failed to fetch data from the core, e.g. internal plugin-lib methods like get_line have failed. This isn't really an error with conversions, this represents a fundamental IO error, and is probably not recoverable. As above, it isn't even clear that we should be doing any conversions in the lsp-lib.
In core, We send out a get_hover notification, and then we receive a show_hover notification, which contains a Result. If there's an error message, we print it.
but here is easier?
In the lsp-plugin, we get a get_hover notification, and then we send document/hover to the actual language server. If we get an error we print it, and then send it on to core.

At no point do we actually do anything with this error; it doesn't effect any program logic.We would achieve the same result by just printing a log from the LSP client, and then not calling core back at all.

Is this a better approach? That depends on what exactly the error means, and whether or not core can do anything about it.

What might core do with it?

One thing we could imagine is that, when no hover information is available, we might want the client to show an empty hover view with some placeholder text saying "no definition found" or something. We can only know there's no definition after we've heard a response from each plugin; so in this case after sending get_hover (or whatever) to each plugin, we would wait for each plugin to call us back; if all plugins had called us back and we had no definition, we could then call the client and tell it that we had no definition.

The major question is: "what does this error mean, and who is it for?"

I think I'm rambling, but maybe we should have a screenshare and I can try and walk through some of this a bit more?

cmyr · 2018-07-13T15:15:31Z

        .spawn()
        .expect("Error Occurred");

+    for var in std::env::vars() {


maybe we don't want to commit this?

cmyr · 2018-07-13T15:18:47Z

+#[derive(Serialize, Deserialize, Debug)]
+/// Range to be sent to client specified in utf-8 offsets
+pub struct Range {
+    // Utf-8 offsets will be sent to clients only


it's confusing that we only send utf-8 offsets, but we accept utf-16?

cmyr · 2018-07-13T15:27:19Z

+            .language_server_clients
+            .get(&view_info.ls_identifier)
+            .unwrap();
+


At the very least all of this boilerplate for getting the ls_client can be moved into a helper function.

cmyr · 2018-07-13T15:28:23Z

                        let path = view.get_path().clone().unwrap();
-                        get_workspace_root_uri(workspace_identifier, path).ok()
+                        let q = get_workspace_root_uri(workspace_identifier, path);
+                        eprintln!("PATH {:?}", q);


in general there are a lot of print statements in this PR. Some print statements are fine, especially when they are used to report unexpected behaviour, but too much printing in routine situations can end up just cluttering the logs.

cmyr · 2018-07-13T15:31:01Z

+    pub end: usize
+}
+
+#[derive(Serialize, Deserialize, Debug)]


we haven't been super consistent here, but our preference is for /// comments to come before #[annotations].

cmyr · 2018-07-13T17:34:17Z

+            .get(&view_info.ls_identifier)
+            .unwrap();
+
+        let position_ls = lsp_position_from_core_position(view, position);


Didn't we decide to do this conversion in core? We shouldn't need to have conversion logic in both core and in the plugin lib. (see my top level comment)

cmyr · 2018-07-13T17:35:30Z

+            }
+            Err(error) => {
+                eprintln!("Can't convert location to offset. Error {:?}", error);
+                let err = LanguageResponseError::PositionConversionError(format!("{:?}",error));


I would prefer to impl From<_> to do this conversion, and then we could just have ls_client.core.display_hover(..., err.into()); although I have bigger questions about this, see my top level comment.

cmyr · 2018-07-13T17:57:05Z

Oh, one more thing:

I realize what the problem is about linebreaks and line/col. The client sends line/col that include soft breaks; however each language server plugin has its own copy of the document, and these don't include soft breaks. When we send a position to the language server plugin, we need to ensure that it is only using hard breaks; this means that we should be calling view methods to get the position for the plugin, we should only be calling methods on the Rope.

Similarly, when we get a position back from the plugin, we need to avoid using the view methods when we convert that position, or we will be accidentally taking the soft wraps into account and will get incorrect results again.

betterclever · 2018-07-23T15:43:27Z

Closing in favour of #753

googlebot added the cla: yes label Jun 19, 2018

betterclever force-pushed the hover-support branch 3 times, most recently from b407b8b to 96f95de Compare June 21, 2018 17:54

betterclever force-pushed the hover-support branch from 96f95de to 535f8b6 Compare June 21, 2018 18:19

betterclever changed the title ~~WIP: Add Hover Definition Request Support~~ WIP: Add Hover + Go-to Definition Request Support Jun 28, 2018

cmyr reviewed Jul 9, 2018

View reviewed changes

betterclever force-pushed the hover-support branch 3 times, most recently from b51983c to 41055f2 Compare July 10, 2018 15:11

cmyr reviewed Jul 10, 2018

View reviewed changes

betterclever force-pushed the hover-support branch 2 times, most recently from 3ce7482 to 626bad2 Compare July 11, 2018 01:54

betterclever changed the title ~~WIP: Add Hover + Go-to Definition Request Support~~ Add Hover + Go-to Definition Request Support Jul 11, 2018

betterclever force-pushed the hover-support branch from 626bad2 to b2340f9 Compare July 11, 2018 12:35

cmyr reviewed Jul 11, 2018

View reviewed changes

betterclever force-pushed the hover-support branch 2 times, most recently from 5086343 to d186bbb Compare July 11, 2018 16:52

betterclever added 6 commits July 12, 2018 16:08

Added Initialize method to Plugin Trait

247141f

Initial Stage Hover RPC

6938e86

Conversion of LSP format position to Utf8 Offset

bab036d

Added Client RPC Handling for hover

60f30e6

Converted Markup Content to Markdown

cecf5b5

fixed hover request handling logic + forwarded result to client

8b0ef38

betterclever added 8 commits July 12, 2018 16:08

Added definition support in Client Plugin

5b02583

Handled document close in LSP Plugin

cd2161d

Added Typescript config in LSP Plugin

6e32d7e

Set tracing from Language Server to Messages.

e6daf8c

Refactor: Moved conversion functions to conversion_utils

0499877

Response from LSP Plugin made to be a Result

42b987b

Code cleanup

11f5be5

Forwarded definition response to client

d50e2f6

betterclever force-pushed the hover-support branch 2 times, most recently from 1bc8f19 to f28410e Compare July 12, 2018 10:45

betterclever added 2 commits July 12, 2018 16:25

Added RLS build log notification handling in Language Server Client

c6e9b62

Cleanup + Fixes

9b582ed

betterclever force-pushed the hover-support branch from f28410e to 9b582ed Compare July 12, 2018 10:58

Added frontend docs for hover

2b26928

nangtrongvuon mentioned this pull request Jul 13, 2018

Added frontend UI for Hover and Go To Definition xi-editor/xi-mac#221

Closed

cmyr reviewed Jul 13, 2018

View reviewed changes

nangtrongvuon mentioned this pull request Jul 18, 2018

Added hover UI for front end xi-editor/xi-mac#228

Merged

This was referenced Jul 18, 2018

Plugins and the representation of positions (utf8 and utf16, absolute and line/column) #746

Open

Added hover support #753

Merged

betterclever closed this Jul 23, 2018

		}

		fn request_hover(&mut self, request_id: usize, position: Option<Position>) {


		fn request_hover(&mut self, request_id: usize, position: Option<Position>) {

		let position = position.or_else(\|\| self.view.borrow().get_caret_offset()

		}

		fn request_defintion(&mut self, request_id: usize, position: Option<Position>) {

		}


		pub fn offset_to_line_col_utf16(&self, text: &Rope, offset: usize) -> (usize, usize) {


		fn config_changed(&mut self, _view: &mut View<Self::Cache>, _changes: &ConfigTable) {}

		fn get_hover(

Conversation

betterclever commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

betterclever commented Jun 21, 2018

Uh oh!

nangtrongvuon commented Jun 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

betterclever commented Jun 24, 2018

Uh oh!

nangtrongvuon commented Jun 26, 2018

Uh oh!

cmyr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmyr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

betterclever Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

betterclever commented Jun 19, 2018 •

edited

Loading

nangtrongvuon commented Jun 24, 2018 •

edited

Loading

cmyr left a comment •

edited

Loading

betterclever Jul 11, 2018 •

edited

Loading