Some optimization

[titoBouzout]

I've added some optimizations but basically is the same. I no tested it to much.

[titoBouzout]

One thing, I was unable to understand if the previous regexp_escape was doing some special or was just a regexp_escape.. I replaced it by built.in re.escape.

[barneywilliams]

Tito,

You have access to those repos in the S2 org on GitHub. All I do is admin. It is up to devs to push/merge their own changes in.

Greg

"You have enemies? Good. That means youve stood up for something, sometime in your life." ~ Winston Churchill

On Fri, Oct 28, 2011 at 1:49 PM, tito < reply@reply.github.com>wrote:

I've added some optimizations but basically is the same. I no tested it to much.

You can merge this Pull Request by running:

git pull https://github.com/titoBouzout/WordHighlight master

Or you can view, comment on it, or merge it online at:

https://github.com/SublimeText/WordHighlight/pull/5

-- Commit Summary --

• Basically the same

-- File Changes --

M word_highlight.py (77)

-- Patch Links --

https://github.com/SublimeText/WordHighlight/pull/5.patch https://github.com/SublimeText/WordHighlight/pull/5.diff

Reply to this email directly or view it on GitHub: https://github.com/SublimeText/WordHighlight/pull/5

[ghost]

Changes work alright for me, and the code overall looks better now (thank you). Will have to wait on adzenith's response about the regex, though.

[adzenith]

Regarding the regex stuff: there was something about re.escape that didn't work right when I was trying it. I don't remember exactly what it was, but there seemed to be some stuff it wouldn't escape correctly. My solution I believe works in all cases.

Regarding the pull request: It would make me much happier if a commit that seems to change about 90% of the file had a commit message of something other than "Basically the same". Can this commit be broken into multiple commits in any way? Can you summarize what you've done in the commit message? Also, one nit-picky thing (sorry): is there a reason you changed the variable "settings" to "s"? I feel its purpose was much clearer before. The add_on_change thing is cool, though. I didn't know you could do that.

[adzenith]

I've been playing around with re.escape and I can't figure out why I didn't use it. I could have sworn there was something about it—I remember feeling bitter because re.escape didn't actually escape the regex right. Who knows!? Weird.

[ghost]

Regarding the re-added delay: it doesn't work properly with values <0.05sec under Linux, at least under my setup. If you happen to know the way to rewrite the related portion with sublime.set_timeout and some sort of "clear_timeout" instead of python threads, the performance would benefit from that too (I'm not sure it is possible, though: http://www.sublimetext.com/forum/viewtopic.php?f=6&t=3406).

[titoBouzout]

Regarding the regex stuff: there was something about re.escape that didn't work right when I was trying it. I don't remember exactly what it was, but there seemed to be some stuff it wouldn't escape correctly. My solution I believe works in all cases.

Ok, lets try with this one until someone hit the bug again and we can look what's going on. The idea is to run fast as possible.

Can you summarize what you've done in the commit message?

Sorry for the large commit, I needed the full version working and tested many variations.

Basically is the same heart but optimized.

The main code of this plugin runs _on_selectionmodified this means every time the cursor is moved via click, dblclick, typing, dragging for selecting, or deleting this functions runs, hundred of times in a few seconds. Everything in there should be optimized to max.

If you access information that is not going to change, you may want to cache that info into an object with direct access.

In a listener like this, you also want to do the few amount of operations possibles

In settings we avoid to ask sublime, for data with key "x" about file "y" hundred of times by setting these previously. I also changed the conditions to get the same results but with faster calculations

Instead of this:

if len(sel) < 200 and len(sel) == len(view.word(sel)):

• computing len of sel
• computing len of sel again
• getting word from sel
• computing len of word
• check if len of sel < 200
• check if len of len word = sel

this

if word.end() == sel.end() and word.begin() == sel.begin() :

check if number == number check if number == number

I also noticed that the regions were added but not removed. There is an API to remove regions. If it's there, there must be a reason. Another thing was that the author was calling add_regions without checking if there is regions to add. Will join the party an empty list?.

is there a reason you changed the variable "settings" to "s"? I feel its purpose was much clearer

Nothing special, it just don't fit into my monitor if I write the complete word : ) I'l change it back.

[titoBouzout]

Regarding the re-added delay: it doesn't work properly with values <0.05sec under Linux, at least under my setup. If you happen to know the way to rewrite the related portion with sublime.set_timeout and some sort of "clear_timeout" instead of python threads, the performance would benefit from that too (I'm not sure it is possible, though: http://www.sublimetext.com/forum/viewtopic.php?f=6&t=3406).

That value is just symbolic. There is no real difference. we should change it to 0.05 to get it working on Linux?

The small visible delay when dblclicking is because the re-queue of the operation via sublime.set_timeout.

We can fix this by checking if at least there is some text selected to start queuing calls. Then the first highlight in theory should run on same thread and be instantaneous. complex.

[adzenith]

Tito: I'm planning on merging the first commit right now, but rewording it to read "Rework entire file to be more efficient". Is that okay? For the second commit, I might hold off until we get the timing thing figured out. What do you think? I'm not sure how I feel about the last commit because you're also changing the default settings to a behaviour that I feel is more surprising to users of the plugin, that is, you're turning on highlighting when the selection is empty. Is that intended to be in the commit? It's not referenced in the message.

[titoBouzout]

Tito: I'm planning on merging the first commit right now, but rewording it to read "Rework entire file to be more efficient". Is that okay?

Great ! :)

For the second commit, I might hold off until we get the timing thing figured out. What do you think?

I think the second commit should go with the first one. When selecting text that listener works hard.

you're also changing the default settings

Change it back to default please, I missed that, maybe on testing.

Thank you!

[titoBouzout]

I remember about the settings. I take these from the readme.

Please take your time, any question let me know.

[adzenith]

Done. Thanks a bunch for all your work!