Giter Club home page Giter Club logo

Comments (13)

sigmaSd avatar sigmaSd commented on August 19, 2024 1

@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.

sigmaSd avatar sigmaSd commented on August 19, 2024 1

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.

sigmaSd avatar sigmaSd commented on August 19, 2024 1

#47

from irust.

sigmaSd avatar sigmaSd commented on August 19, 2024

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.

sigmaSd avatar sigmaSd commented on August 19, 2024

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.

smolck avatar smolck commented on August 19, 2024

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.

sigmaSd avatar sigmaSd commented on August 19, 2024

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.

sigmaSd avatar sigmaSd commented on August 19, 2024

@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.

smolck avatar smolck commented on August 19, 2024

@sigmaSd Great, thanks! I should be able to get a PR today if all goes well.

from irust.

smolck avatar smolck commented on August 19, 2024

@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.

sigmaSd avatar sigmaSd commented on August 19, 2024

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.

smolck avatar smolck commented on August 19, 2024

@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.

sigmaSd avatar sigmaSd commented on August 19, 2024

@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)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo 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.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.