-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you for adding this :)
One issue I have encountered is that it needs to be made compatible with the new WASM-based client as well as the desktop client. Could you look into whether you could adapt it to use this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment.
Increased perfomance by storing layout job so it is recreated every frame. It is only recreated when code is change which is checked by comparing hashes
codectrl-gui/src/components/details_view_components/information_grid.rs
Outdated
Show resolved
Hide resolved
|
||
let ps = SyntaxSet::load_defaults_newlines(); | ||
let ts = ThemeSet::load_defaults(); | ||
let syntax = ps.find_syntax_by_extension("py").unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you instead get the extension of the log file name?
codectrl-gui/src/components/details_view_components/code_highlighting.rs
Outdated
Show resolved
Hide resolved
codectrl-gui/src/components/details_view_components/information_grid.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one more thing and it should be fine to merge, after I test it.
pub fn code_highlighter(code: &str, lang: &str) -> egui::text::LayoutJob { | ||
let ps = SyntaxSet::load_defaults_newlines(); | ||
let ts = ThemeSet::load_defaults(); | ||
let syntax = ps.find_syntax_by_extension("py").unwrap(); | ||
let syntax = ps.find_syntax_by_extension(lang_to_short(lang)).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.rs/syntect/4.6.0/syntect/parsing/struct.SyntaxSet.html#method.find_syntax_by_name
You should be able to use that and just pass lang
into it, rather than creating a new function for it.
Something like this:
let syntax = ps.find_syntax_by_name(lang).unwrap();
fn lang_to_short(lang: &str) -> &str { | ||
// Add more language here | ||
match lang { | ||
"rust" => "rs", | ||
"python" => "py", | ||
_ => "", | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at previous comment to see why possibly unnecessary.
Syntax definition search goes from language specified by the logger -> the extension of the file -> plaintext.
Completed code highlighting for python and rust for now. I haven't added language option to the schema. Current the highlighting is hard-coded to rust should be able to change that when all the loggers start to sent the language.