Skip to content

Use faster-than-regex .substring() slicing#84

Merged
jorgebucaran merged 2 commits intomainfrom
substring
Oct 3, 2021
Merged

Use faster-than-regex .substring() slicing#84
jorgebucaran merged 2 commits intomainfrom
substring

Conversation

@jorgebucaran
Copy link
Copy Markdown
Owner

This improves runtime performance when clearing bleeding sequences, using .substring() rather than regexes.

This clever idea is inspired by @alexeyraspopov's replaceClose function in picocolors. Thank you for that! 🙌

Improve runtime performance when clearing bleeding sequences.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #84 (57d4fad) into main (eba26cd) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #84   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          133       147   +14     
=========================================
+ Hits           133       147   +14     
Impacted Files Coverage Δ
index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eba26cd...57d4fad. Read the comment docs.

@ai
Copy link
Copy Markdown
Contributor

ai commented Oct 2, 2021

Add mentions about inspiration to docs?

@kibertoad
Copy link
Copy Markdown
Collaborator

@jorgebucaran I think that's a good idea. Let's add a section dedicated to credits and honourable mentions.

@jorgebucaran jorgebucaran added the enhancement New feature or request label Oct 3, 2021
@jorgebucaran jorgebucaran merged commit 0233765 into main Oct 3, 2021
@jorgebucaran jorgebucaran deleted the substring branch October 3, 2021 03:16
jorgebucaran added a commit that referenced this pull request Oct 3, 2021
@jorgebucaran
Copy link
Copy Markdown
Owner Author

That's a great idea. Done in 6a4a871! 💯

@ai
Copy link
Copy Markdown
Contributor

ai commented Oct 3, 2021

@jorgebucaran 6a4a871 will be better by adding explicit link to chalk and picocolors’s mention and link.

@jorgebucaran
Copy link
Copy Markdown
Owner Author

I'm pretty happy with the result, actually, but I'll definitely consider your suggestion. Thanks!

@alexeyraspopov
Copy link
Copy Markdown

The acknowledgment sounds like it was my contribution to this project, which it wasn’t.

I don’t mind adopting performance tricks.

I would really appreciate if you could update this section with proper links to the projects. Would be even better if the section wasn’t buried somewhere down the page. Thanks

@jorgebucaran
Copy link
Copy Markdown
Owner Author

Sure, I can think of another way to acknowledge you. 👌

@kibertoad
Copy link
Copy Markdown
Collaborator

@alexeyraspopov We ended up listing reference in release notes next to a specific change, which seems more appropriate, since this is a one-off optimization and not something fundamental to how Colorette is built.
Agreed on readme notes making it sound like it was contributed by you directly. Would you prefer us rephrasing, or removing it and just keeping release notes?

@alexeyraspopov
Copy link
Copy Markdown

I’d prefer adding links to the projects, as I mentioned previously.

@kibertoad
Copy link
Copy Markdown
Collaborator

Yup, we are adding links for sure, give us a bit of time :)

@jorgebucaran
Copy link
Copy Markdown
Owner Author

Absolutely. Added in ada9fb3. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants