Skip to content

Include plugin name with AddStatusItem RPC#725

Merged
cmyr merged 4 commits intoxi-editor:masterfrom
nangtrongvuon:statusitem-pluginid
Jul 3, 2018
Merged

Include plugin name with AddStatusItem RPC#725
cmyr merged 4 commits intoxi-editor:masterfrom
nangtrongvuon:statusitem-pluginid

Conversation

@nangtrongvuon
Copy link
Copy Markdown
Member

@nangtrongvuon nangtrongvuon commented Jun 23, 2018

This PR makes adding the status item include the plugin name from which it originates, so that client can manage items that come from the same plugin.

companion to xi-mac #210

Copy link
Copy Markdown
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usual nits.

This also needs an update to the documentation, and now that we've merged the xi-mac PR we can update the API there as well. 😄

use serde_json::{self, Value};
use xi_rpc::{self, RpcPeer};

use tabs::ViewId;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this from rustfmt or something? Normally we don't include braces on a single-item import.

Comment thread rust/core-lib/src/client.rs Outdated
}

pub fn add_status_item(&self, view_id: ViewId, key: &str, value: &str, alignment: &str) {
pub fn add_status_item(&self, view_id: ViewId, plugin: &str, key: &str, value: &str, alignment: &str) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this a bit more, and I think instead of calling this plugin we should call it source. I think it's likely that core will end up using this API, so we should be more general.

Comment thread rust/core-lib/src/event_context.rs Outdated
self.view_id, &key, &value, &alignment),
AddStatusItem { key, value, alignment } => {
let plugin_name = &self.plugins.iter().find(|p| p.id == plugin).unwrap().name;
self.client.add_status_item(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting here is a bit messed up.

@@ -384,6 +384,8 @@ this writing, the following is valid json for a `Command` object:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the signature for the method needs to be updated as well.

Comment thread rust/core-lib/src/TEST Outdated
@@ -0,0 +1 @@
2. Press v and move the cursor to the fifth item below. Notice that the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@cmyr
Copy link
Copy Markdown
Member

cmyr commented Jul 3, 2018

@nangtrongvuon can you take another pass over this, and then I'll take a look?

Copy link
Copy Markdown
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Copy link
Copy Markdown
Member

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants