Add *.md support for plugin's details #2204
Conversation
There was a problem hiding this comment.
This looks great.
We will still need to deliver the path of the md file from the backend, to keep all loading logic in one place and having the front end knowing less as possible.
I'll create another PR for that once I'm done with changes on the plugins manager
DmitryAstafyev
force-pushed
the
plugins_support
branch
from
88c9767 to
e12330e
Compare
There was a problem hiding this comment.
Great work nicely done @DmitryAstafyev
I still found some points mainly regarding the naming of some fields like ts or rd. Also we may need to extend the proptests and protocol test
| @@ -90,19 +93,24 @@ async fn load_plugin( | |||
| plug_dir: PathBuf, | |||
| plug_type: PluginType, | |||
| ) -> Result<PluginEntityState, InitError> { | |||
| let mut rd = PluginRunData::default(); | |||
There was a problem hiding this comment.
I really like the names to be more expressive. Can we change this to run_data or rdata
| pub fn installed_plugins(&self) -> &[PluginEntity] { | ||
| &self.installed_plugins | ||
| pub fn installed_plugins(&self) -> Vec<&PluginEntity> { | ||
| self.installed_plugins | ||
| .iter() | ||
| .map(|en| en.into()) | ||
| .collect::<Vec<&PluginEntity>>() |
There was a problem hiding this comment.
We can return an iterator here without having to collect into vector inside the function
/// Provide full infos of all loaded and valid plugins.
pub fn installed_plugins(&self) -> impl Iterator<Item = &PluginEntity> {
self.installed_plugins.iter().map(|en| en.into())
}| pub fn invalid_plugins(&self) -> &[InvalidPluginEntity] { | ||
| &self.invalid_plugins | ||
| pub fn invalid_plugins(&self) -> Vec<&InvalidPluginEntity> { | ||
| self.invalid_plugins | ||
| .iter() | ||
| .map(|en| en.into()) | ||
| .collect::<Vec<&InvalidPluginEntity>>() |
There was a problem hiding this comment.
Returning an iterator is more flexible here as well
pub fn invalid_plugins(&self) -> impl Iterator<Item = &InvalidPluginEntity> {
self.invalid_plugins.iter().map(|en| en.into())
}| /// The core entity representing the plugin. | ||
| pub entity: PluginEntity, | ||
| /// Runtime data associated with the plugin. | ||
| pub rd: PluginRunData, |
There was a problem hiding this comment.
Can we please rename this to more meaningful name like run_data.
I think rust LSP is really efficient for mass renaming 😉
| /// The core entity representing an invalid plugin. | ||
| pub entity: InvalidPluginEntity, | ||
| /// Runtime data associated with the invalid plugin. | ||
| pub rd: PluginRunData, |
| pub fn debug<S: AsRef<str>>(msg: S) -> Self { | ||
| Self { | ||
| msg: msg.as_ref().to_string(), | ||
| level: PluginLogLevel::Debug, | ||
| tm: Self::tm(), | ||
| } | ||
| } | ||
| pub fn info<S: AsRef<str>>(msg: S) -> Self { | ||
| Self { | ||
| msg: msg.as_ref().to_string(), | ||
| level: PluginLogLevel::Info, | ||
| tm: Self::tm(), | ||
| } | ||
| } | ||
| pub fn err<S: AsRef<str>>(msg: S) -> Self { | ||
| Self { | ||
| msg: msg.as_ref().to_string(), | ||
| level: PluginLogLevel::Err, | ||
| tm: Self::tm(), | ||
| } | ||
| } | ||
| pub fn warn<S: AsRef<str>>(msg: S) -> Self { | ||
| Self { | ||
| msg: msg.as_ref().to_string(), | ||
| level: PluginLogLevel::Warn, | ||
| tm: Self::tm(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Also the optional removing generic bound with deref coercion + Short docs
| /// The severity level of the log message. | ||
| pub level: PluginLogLevel, | ||
| /// The timestamp of when the log message was generated, represented as a Unix timestamp. | ||
| pub tm: u64, |
There was a problem hiding this comment.
Also renaming this to timestamp would be great 🙏
| dir_path, | ||
| plugin_type, | ||
| error_msgs, | ||
| }) | ||
| .boxed() | ||
| } |
There was a problem hiding this comment.
I think we need a proptest for PluginRunData here
| @@ -137,6 +137,7 @@ gen_encode_decode_fns!(PluginConfigValue); | |||
| gen_encode_decode_fns!(PluginConfigSchemaType); | |||
| gen_encode_decode_fns!(PluginConfigSchemaItem); | |||
| gen_encode_decode_fns!(PluginEntity); | |||
| gen_encode_decode_fns!(PluginRunData); | |||
There was a problem hiding this comment.
I think we need to extend the tests in ts-bindings/spec/session.protocol.spec.ts
| @@ -154,6 +155,7 @@ gen_encode_decode_fns!(CommandOutcome<InvalidPluginsList>); | |||
| gen_encode_decode_fns!(CommandOutcome<PluginsPathsList>); | |||
| gen_encode_decode_fns!(CommandOutcome<Option<PluginEntity>>); | |||
| gen_encode_decode_fns!(CommandOutcome<Option<InvalidPluginEntity>>); | |||
| gen_encode_decode_fns!(CommandOutcome<Option<PluginRunData>>); | |||
There was a problem hiding this comment.
Protocol tests in ts-bindings here as well.
Added
README.mdfile in plugin's folder.README.mdhas been found, it would be rendered in details frameChange
PluginRunDataentity to store run, load and checks data