Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Highlight Only Visible Area (performance improvement) #31

Open
Moosems opened this issue May 1, 2023 · 5 comments · May be fixed by #35
Open

Highlight Only Visible Area (performance improvement) #31

Moosems opened this issue May 1, 2023 · 5 comments · May be fixed by #35
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed

Comments

@Moosems
Copy link
Collaborator

Moosems commented May 1, 2023

This will fix #9 and will fix #11. The idea? Highlight only text that's visible. This will be done by changing over to a method called .highlight() that will highlight all the visible text with reference to all the text in the widget. The structure of the method will be roughly as follows:

  1. Get the visible area
  2. Remove all tags
  3. Get all the text in the widget
  4. Lex the lines and use offsets
  5. Find what is not visible at all and discard those
  6. Modify remaining tags to cut out parts outside of view area
  7. Apply what's left

@rdbende What do you think? I have a feeling this will have a performance improvement for large amounts of code and and that small amounts will still be fine.

@Moosems
Copy link
Collaborator Author

Moosems commented May 1, 2023

Can also fix #5 by checking if emoji's are in the tags and removing those areas.

@Moosems
Copy link
Collaborator Author

Moosems commented May 1, 2023

[12:05]	Moosems	Did you see my idea for the visible area highlighting?
[12:07]	rdbende	What do you mean by the 3rd point?
[12:07]	Moosems	"Get all the text in the widget
[12:07]	Moosems	"?
[12:07]	rdbende	Yes
[12:07]	rdbende	Why we need all the text?
[12:08]	Moosems	For lex context (we're not applying tags everywhere but it will fix docstring issues and whatnot that may require multiple lines of context)
[12:09]	rdbende	I see
[12:09]	rdbende	It would be quite expensive though
[12:10]	Moosems	How would you go about tackling the docstring issue then?
[12:10]	Moosems	Cause you're absolutely right, it would be a lot more CPU expensive
[12:11]	rdbende	I have no better idea, but it would be slow to lex the entire textwidget on every keypress
[12:11]	Moosems	A background runner that runs on a seperate thread?
[12:12]	rdbende	Not a fan of that idea tbh
[12:14]	Moosems	We can start by having it only get the text in the viewable lines (get columns to the right still)
[12:18]	*	BH23 quit. (Ping timeout: 268 seconds)
[12:19]	*	biberao joined ##learnpython.
[12:21]	Moosems	Y/N?
[12:22]	biberao	hi
[12:23]	rdbende	Moosems let's start with that
[12:23]	rdbende	Hi biberao!
[12:23]	biberao	sup
[12:24]	Moosems	Hi biberao!
[12:25]	Moosems	rdbende: We'll figure out the docstrings and whatnot eventually
[12:26]	rdbende	Have any of you ever tried customtkinter?
[12:27]	rdbende	Just wondering, what's all that hype around it
[12:27]	rdbende	It looks horrible outside macOS
[12:27]	Moosems	I looked at it once but not very much
[12:28]	biberao	lets do some py togetherrrrrrrrr
[12:28]	Moosems	biberao: Deal. How would you implement multiline highlighting on somethign requiring ourside context without wasting a ton of CPU time?
[12:29]	biberao	never tried
[12:29]	Moosems	rdbende: Maybe checking for items that call for multiple lines of context (like """), finding the first linked one, and just adding that and offsetting it?
[12:30]	Moosems	So if visible consists of:
[12:30]	Moosems	"""
[12:30]	Moosems	print("Hello, world")
[12:30]	Moosems	and before this there's a """
[12:30]	Moosems	Then it will find it knowing its Python and add it to the start
[12:31]	rdbende	And doing that for every possible language?
[12:32]	Moosems	Does pygments have a value for multiline items?
[12:32]	Moosems	If so, we could steal it from there
[12:32]	rdbende	Idk, need to look it up
[12:34]	Moosems	If we find a Token as String.Doc we can search around maybe?
[12:35]	Moosems	Or Comment.Multiline
[12:39]	biberao	but im willing to learn
[12:44]	Moosems	Ok so I just proposed the issue to my friends and they were like "IDK this is above my paygrade"
[12:49]	Moosems	rdbende: would a context finding regex be faster to determine how to add it rather than lexing everything?
[12:49]	Moosems	However that would work
[12:50]	rdbende	Persumably yes
[12:50]	Moosems	I have an idea. Imagine the following edge case:
[12:50]	rdbende	Pygments also uses regexes for lexing, so one instead of many will be faster
[12:51]	*	cthlolo joined ##learnpython.
[12:51]	Moosems	Thats what I was thinking
[12:52]	Moosems	Let us consider the following edge case:
[12:52]	Moosems	https://dpaste.com/57QZ43DJ2
[12:53]	Moosems	If we know that there is a token with MLC or DS, we know we should look for outside context
[12:53]	Moosems	OOOOOOO I have an idea!!
[12:53]	rdbende	<3
[12:54]	Moosems	Tags that aren't tokens that remember where DS and MLC's are that don't get deleted and we only look at those! :)))
[12:54]	Moosems	Because we only delete tokens starting with "Token"
[12:54]	rdbende	I like it!
[12:55]	Moosems	:D
[12:56]	Moosems	Then we make pairs of starts and end, and if there's a start end, or full pair inside of the viewport we can add needed context outside :)
[12:56]	Moosems	Then the question becomes how to add that proper context
[12:59]	Moosems	What do you think of that problem rdbende?
[13:01]	rdbende	If it finds an end, it searches for a start above, and highlights from there
[13:02]	Moosems	And what if I'm trying to be annoying and make a really really long docstring with code inside?
[13:02]	rdbende	Yes, i turned off notifications yesterday, so that's why you need to ping me all the time
[13:02]	Moosems	Does pygments ignore code lexing inside of docstrings>
[13:03]	rdbende	Yes, it does
[13:03]	Moosems	notifications in mantaray or github?
[13:03]	rdbende	In mantaray
[13:03]	rdbende	But i turned them back on
[13:03]	Moosems	rdbende rdbende rdbende :D
[13:03]	*	rdbende quit. (Remote host closed the connection)
[13:03]	*	rdbende joined ##learnpython.
[13:03]	Moosems	rdbende rdbende rdbende :D
[13:04]	Moosems	Lol
[13:04]	rdbende	Now I disabled audio notification
[13:04]	rdbende	So it doesn't get recorded :D
[13:04]	*	Moosems sideyes rdbende
[13:04]	Moosems	:D
[13:05]	Moosems	I made a special audio notification a while back
[13:06]	Moosems	We can check if we should add an MLCDS tag if a line starts or ends with a MLC or DS right?
[13:07]	rdbende	MLCDS XDDD
[13:07]	rdbende	Yes
[13:07]	rdbende	But no the token tag, right?
[13:07]	Moosems	NOT a Token tag
[13:07]	Moosems	That way it doesn't get deleted
[13:07]	rdbende	Yupp
[13:07]	rdbende	Ok
[13:07]	rdbende	MLCDS
[13:08]	Moosems	MLCDS is the name XD

@Moosems
Copy link
Collaborator Author

Moosems commented May 8, 2023

Ok, I have started a dev branch for this and now need to flesh out the idea fully.

@Moosems Moosems linked a pull request May 8, 2023 that will close this issue
@Moosems Moosems added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed labels May 8, 2023
@Moosems
Copy link
Collaborator Author

Moosems commented Jul 12, 2023

To determine what starts a MLCDS we can take one in the file and lex it character by character until the tag occurs, mark that as a start and then find the end of one (if any) and then subtract characters until the end disappears and then take the removed characters and mark those as the end. Once those are known, it makes the future much simpler.

@Moosems
Copy link
Collaborator Author

Moosems commented Jul 12, 2023

Markdown with backsticks and a second language will be difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant