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

Highlighter#withGutter() should be a real "wither" #99

Closed
Kocal opened this issue Apr 13, 2024 · 3 comments
Closed

Highlighter#withGutter() should be a real "wither" #99

Kocal opened this issue Apr 13, 2024 · 3 comments

Comments

@Kocal
Copy link

Kocal commented Apr 13, 2024

Hi,

First let me thank you and every contributors for this package. I first went with Shiki.js for code-highlighting in the browser, but I found that was not so much performant for my needs, too much JavaScript was loaded, and I suspect it badly impacted the page interactivity. To me, a server-based solution is so much better because I can use HTTP cache for more optimisation! :)


I've just found Highlighter#withGutter() is in fact not a real "wither" , even if its name starts with with.

"Wither" methods are special methods that clones the object, mutate it, and returns the cloned object, this way the original object stays untouched. We can find many exemples in the PHP ecosystem:

So we have two solutions:

  1. Either make it a real wither, the object is cloned, modified, and returned
  2. Either we rename the method to enableGutter, or something else that will says to users "this will not returns a new object".

In both cases, to not introduce breaking changes, we can trigger a deprecation that will be removed in the next major version (like Symfony does).

WDYT?

@brendt
Copy link
Member

brendt commented Apr 15, 2024

Agree on with being confusing. I'm already working on a v2 branch, so I'll include it in there :)

@brendt
Copy link
Member

brendt commented Apr 15, 2024

Prepared in v2: #93

@brendt brendt closed this as completed Apr 15, 2024
@Kocal
Copy link
Author

Kocal commented Apr 15, 2024

Perfect, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants