Comments (13)
@smolck thanks a lot for opening this issue!
1- highlighting the buffer and then printing it with syntect actually works but its too slow, this is the best method if we actually figure how to speed it up
2- the printer abstraction is bad, just leagacy code, I doubt it can help much
3- I like highlighting keywords manually, this can be the easiest and effective approach indeed, instead of rellying on syntect, we use a custom highliht function
from irust.
Btw if you want any help on this Im available at crossterm discord server https://discord.gg/p4NGQf, @mrcool, we can discuss it in offtopic channel
from irust.
from irust.
Also now that I think about 3, we cant rely on the space because the user can add a character inside the line, so logically we have to evaluate the whole buffer with each input.
Printing the whole buffer with each input is not actually costy(there is no percievable performance hit) , the issue is once again is coloring, so we're back to 1
Intuitevly I think Im not using syntect correctly since highlighting shouldnt be slow at all I feel this is the best front to tackle, maybe use once_cell for the first 4 lines here https://github.com/sigmaSd/IRust/blob/master/src/irust/highlight.rs#L7 instead of creating them each time, I did not test it but maybe that can improve performance
from irust.
If we're still facing issues with syntect we can just create a simple custom parser that highlights keywords as you suggested, should be fun to make!
Just saying Im leaving this for you @smolck you can make a Pr, ping me for infos, or just tell me you're through with it xD either way Im not in a hurry at all, this is a cool feature to have but it doesnt block other work, so Ill keep it open till you're done
from irust.
Just saying Im leaving this for you @smolck you can make a Pr, ping me for infos, or just tell me you're through with it xD either way Im not in a hurry at all, this is a cool feature to have but it doesnt block other work, so Ill keep it open till you're done
Thanks a lot! Hopefully the implementation won’t take too long.
the printer abstraction is bad, just leagacy code, I doubt it can help much
Where all is the printer used (I haven’t looked into it)? Should it be removed/replaced or similar (different issue I imagine, but I’m interested in what you want to do with it)? Having not looked at the code, could that be the bottleneck?
Intuitevly I think Im not using syntect correctly since highlighting shouldnt be slow at all I feel this is the best front to tackle, maybe use once_cell for the first 4 lines here https://github.com/sigmaSd/IRust/blob/master/src/irust/highlight.rs#L7 instead of creating them each time, I did not test it but maybe that can improve performance
Okay, I’ll try that out first. Forgive my ignorance, but you mentioned using once_cell
for the first 4 lines; I’m not familiar with that, care to elaborate?
I may look into how text editors (xi editor
for example) and other projects use syntect to highlight. It’s supposed to be fast (from the README), so I’ll look into how to use it optimally.
Overall, thanks @sigmaSd for welcoming me to the project. Makes contributing much smoother!
from irust.
You're very welcome!
Printer is just used to print output depending on what it is: command, shell.. , it doesnt do anything special its juat a way to organize the code, actually I guess IRust code base is really underdocumented so Ill probably focus on docs next,
once_cell
https://docs.rs/once_cell/0.2.3/once_cell/ is like lazy_statics if you're familiar with it, a crate to create global statics, here is an example of usage https://github.com/sigmaSd/IRust/blob/master/src/irust/cargo_cmds.rs#L10, so Im sugessting to make the first variables in highlight function statics so we dont create them each time
from irust.
@smolck I tried it and indeed once_cell
improve the performance of highlighting to very acceptable levels
So here is a recap of what needs to be done:
1- adding lazy evaluation to https://github.com/sigmaSd/IRust/blob/master/src/irust/highlight.rs
2- making handle character print and highlight the whole buffer with each input, this implies a couple of changes elsewhere in the code base, for this task this snippet from @pzmarzly #32 (comment) can be useful
from irust.
@sigmaSd Great, thanks! I should be able to get a PR today if all goes well.
from irust.
@sigmaSd I'm not sure I understand the project enough to do this, and so if you want to go ahead that's fine by me. But if not, how do you want me to go about highlighting the output after a character is typed (i.e. how to fix the following diff)? I tried the following, and it worked somewhat, but not perfect (it messed up backspace
and enter
):
diff --git a/Cargo.toml b/Cargo.toml
index 76ca31b..b24a878 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,4 +20,3 @@ nix = "0.14.1"
[features]
default = ["highlight"]
highlight = ["syntect"]
-
diff --git a/src/irust/events.rs b/src/irust/events.rs
index 646c063..bd12c42 100644
--- a/src/irust/events.rs
+++ b/src/irust/events.rs
@@ -5,6 +5,8 @@ use crate::irust::{IRust, IRustError};
use crate::utils::StringTools;
use crossterm::ClearType;
+use crate::irust::highlight::*;
+
impl IRust {
pub fn handle_character(&mut self, c: char) -> Result<(), IRustError> {
// clear suggestion
@@ -18,6 +20,7 @@ impl IRust {
// Insert input char in buffer
StringTools::insert_at_char_idx(&mut self.buffer, self.internal_cursor.buffer_pos, c);
+
// update histroy current
self.history.update_current(&self.buffer);
@@ -27,8 +30,9 @@ impl IRust {
// unbound upper limit
self.internal_cursor.current_bounds_mut().1 = self.size.0;
- // Write input char
- self.write_insert(Some(&c.to_string()))?;
+ self.printer = highlight(&self.buffer);
+
+ self.write_out()?;
Ok(())
}
diff --git a/src/irust/highlight.rs b/src/irust/highlight.rs
index b5afe0b..b4c2d9e 100644
--- a/src/irust/highlight.rs
+++ b/src/irust/highlight.rs
@@ -1,20 +1,22 @@
use super::printer::{Printer, PrinterItem, PrinterItemType};
use syntect::easy::HighlightLines;
-use syntect::highlighting::{Color, ThemeSet};
+use syntect::highlighting::{ Color, ThemeSet };
use syntect::parsing::SyntaxSet;
-use syntect::util::LinesWithEndings;
+use syntect::util::{ LinesWithEndings };
-pub fn highlight(c: &str) -> Printer {
- let ps = SyntaxSet::load_defaults_newlines();
- let ts = ThemeSet::load_defaults();
+use once_cell::sync::Lazy;
- let syntax = ps.find_syntax_by_extension("rs").unwrap();
- let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
+static PS: Lazy<SyntaxSet> = Lazy::new(|| SyntaxSet::load_defaults_newlines());
+static TS: Lazy<ThemeSet> = Lazy::new(|| ThemeSet::load_defaults());
+
+pub fn highlight(string: &str) -> Printer {
+ let syntax = PS.find_syntax_by_extension("rs").unwrap();
+ let mut h = HighlightLines::new(syntax, &TS.themes["base16-ocean.dark"]);
let mut printer = Printer::default();
- for line in LinesWithEndings::from(c) {
- h.highlight(line, &ps)
+ for line in LinesWithEndings::from(string) {
+ h.highlight(line, &PS)
.into_iter()
.for_each(|(style, part)| {
let fg_color = match style.foreground {
Also, you mentioned #32 (comment); do you want me to do this refactor, or did you show it for a different reason?
from irust.
You're in the right direction indeed!
here are some fixes to your changes
diff --git a/src/irust/events.rs b/src/irust/events.rs
index 5a94297..cff656b 100644
--- a/src/irust/events.rs
+++ b/src/irust/events.rs
@@ -7,6 +7,9 @@ use crossterm::ClearType;
impl IRust {
pub fn handle_character(&mut self, c: char) -> Result<(), IRustError> {
+ // hide cursor
+ let _ = self.cursor.hide();
+
// clear suggestion
self.clear_suggestion()?;
@@ -30,6 +33,18 @@ impl IRust {
// Write input char
self.write_insert(Some(&c.to_string()))?;
+ // We are doing incremental changes to the codebase
+ // So instead of changing handle_char function we just repaint the buffer
+ // go to lock_pos before printing, lock_pos = starting position
+ // print from the start then restore the cursor to its original position
+ let _ = self.cursor.save_position();
+ self.move_cursor_to(
+ self.internal_cursor.lock_pos.0,
+ self.internal_cursor.lock_pos.1,
+ )?;
+ self.printer = super::highlight::highlight(&self.buffer);
+ let _ = self.write_out();
+ let _ = self.cursor.reset_position();
+
+ // show cursor
+ let _ = self.cursor.show();
+
Ok(())
}
diff --git a/src/irust/highlight.rs b/src/irust/highlight.rs
index b5afe0b..239212c 100644
--- a/src/irust/highlight.rs
+++ b/src/irust/highlight.rs
@@ -1,20 +1,21 @@
use super::printer::{Printer, PrinterItem, PrinterItemType};
+use once_cell::sync::Lazy;
use syntect::easy::HighlightLines;
use syntect::highlighting::{Color, ThemeSet};
-use syntect::parsing::SyntaxSet;
+use syntect::parsing::{SyntaxReference, SyntaxSet};
use syntect::util::LinesWithEndings;
-pub fn highlight(c: &str) -> Printer {
- let ps = SyntaxSet::load_defaults_newlines();
- let ts = ThemeSet::load_defaults();
+static PS: Lazy<SyntaxSet> = Lazy::new(SyntaxSet::load_defaults_newlines);
+static TS: Lazy<ThemeSet> = Lazy::new(ThemeSet::load_defaults);
+static SYNTAX: Lazy<&SyntaxReference> = Lazy::new(|| PS.find_syntax_by_extension("rs").unwrap());
- let syntax = ps.find_syntax_by_extension("rs").unwrap();
- let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
+pub fn highlight(c: &str) -> Printer {
+ let mut h = HighlightLines::new(&SYNTAX, &TS.themes["base16-ocean.dark"]);
let mut printer = Printer::default();
for line in LinesWithEndings::from(c) {
- h.highlight(line, &ps)
+ h.highlight(line, &PS)
.into_iter()
.for_each(|(style, part)| {
let fg_color = match style.foreground {
@@ -28,5 +29,7 @@ pub fn highlight(c: &str) -> Printer {
printer.add_new_line(1);
}
+ // highlight adds a new line for each printer element in the loop
+ // the last newline is undesirable and causes issues with printing the buffer
+ printer.pop();
+
printer
}
diff --git a/src/irust/printer.rs b/src/irust/printer.rs
index 81a7be4..b4de1cd 100644
--- a/src/irust/printer.rs
+++ b/src/irust/printer.rs
@@ -50,6 +50,10 @@ impl Printer {
pub fn is_empty(&self) -> bool {
self.inner.is_empty()
}
+
+ pub fn pop(&mut self) -> Option<PrinterItem> {
+ self.inner.pop()
+ }
}
impl Iterator for Printer {
Whats broken with this:
1- Scrolling when reaching the end of the terminal https://github.com/sigmaSd/IRust/blob/master/src/irust/events.rs#L13
2- Incomplete input handling https://github.com/sigmaSd/IRust/blob/master/src/irust/events.rs#L41
However fixing this 2 remaining issues might be a lot of work, we can try to do incremental changes wherever we can so we don't need to modify a lot of code, (like what you did with handle_character
)
Anyhow I guess the biggest obstacle is probably that code base is currently really messy
Maybe I should try fixing that before anything else xD
from irust.
@sigmaSd Thanks for the help!
It seems that the code base being messy gets in the way of adding new features, fixing bugs, etc.; what do you think of holding off on this for a while until the code base is refactored? Might as well; if we can't easily/simply fix those issues you mentioned without changing up the code base, why not go ahead and do a full refactor (with future features/changes in mind)? Again, I'm happy to help where I can.
If we go that route, I think opening an issue for the refactor to discuss and lay out plans before starting is probably a good idea. You have most of the information about what needs to be refactored (of course), so you should probably be the one to open it; I have some ideas, but since I haven't taken the time to really familiarize myself with the code base, please take my suggestions with a grain of salt ;)
from irust.
@smolck you're definitely right, here is #42 (I'm not really good at writing issues, (or organized code bases for that matter :p ))
from irust.
Related Issues (20)
- Add :cargo command
- Add predefined themes
- Rework how irust server works
- Use `rustyline` or `reedline` for input? HOT 1
- Move theme settings from config.toml to themes config
- Improve Jupyter kernel support HOT 7
- typing twice in windows with crossterm-0.26+ HOT 2
- Add search history feature
- Out: IRust: Are you missing a `;` ? HOT 7
- Can't get to work on Windows HOT 15
- editing functions causes redefinitions HOT 33
- Check statements with cargo check HOT 1
- how make irust works with nightly ? error: toolchain 'stable-x86_64-unknown-linux-gnu' is not installed HOT 2
- Using IRust for live scripting of a host HOT 8
- Custom keybindings? HOT 15
- [Feature Request]: Support for `proc_macro` APIs HOT 7
- [Feature Request]: Ability to load custom `Cargo.toml` HOT 2
- Async repl not working properly HOT 5
- irust v1.50.0 failed to compile HOT 2
- Use `rust-analyser` instead of `racer` for autocompletion HOT 7
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from irust.