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

fix: colours in diff codeblock #67

Merged
merged 4 commits into from
Oct 2, 2023
Merged

Conversation

CoralPink
Copy link
Contributor

Hello!!
I always use mdbook-catppuccin! 🐈‍⬛

After updating to v1.0.0, I noticed that the color theme is not applied correctly to the diff.

All we are doing in this PR is reverting the parameters related to diffs back to what they were before v1.0.0, but I think this will fix this issue.

Could you please confirm this?

Thanks!! ☕

Copy link
Contributor

@nekowinston nekowinston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR.
This is an issue with the highlightjs port that got fixed recently.
The files you edited are generated, please don't edit them; we just need to bump the @catppuccin/highlightjs dependency.

@CoralPink
Copy link
Contributor Author

Ah, I see. I'm beginning to understand!

Is it my understanding that this can be solved by simply rewriting pallete/package.json?

I'm not sure if my understanding is correct, as I can't get the build to work in my local environment...

> [email protected] build
> sass -I node_modules --no-charset --no-source-map catppuccin.scss:../src/bin/assets/catppuccin.css catppuccin-admonish.scss:../src/bin/assets/catppuccin-admonish.css

Error: Can't find stylesheet to import.

1 │ @use "@catppuccin/palette/scss/catppuccin" as catppuccin;
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  catppuccin.scss 1:1  root stylesheet

Error: Can't find stylesheet to import.
  ╷
3 │ @use "@catppuccin/palette/scss/catppuccin" as catppuccin;
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
  catppuccin-admonish.scss 3:1  root stylesheet

@CoralPink
Copy link
Contributor Author

Sorry, this was because I did not npm install.

Also, looking closer, maybe the package is supposed to be updated on release.

Sorry for the strange English!
You can also dismiss this PR if it gets in the way!

Copy link
Contributor

@nekowinston nekowinston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, thanks for adjusting the PR! I think that should be it.

@nekowinston nekowinston requested a review from sgoudham October 1, 2023 12:31
@sgoudham
Copy link
Contributor

sgoudham commented Oct 1, 2023

Hey there 👋, thanks for the pull request!

The changes with the dependency bump look good, could you just help me understand why 051a7ff was reverted in 1d10d5b (#67)? I specified the files because using . was copying over the node_modules directory into the source directory which wasn't great when it came time to cargo publish.

@nekowinston nekowinston dismissed their stale review October 1, 2023 14:13

Reworked.

@CoralPink
Copy link
Contributor Author

Sorry for repeating myself...

In the revert, I thought I had only undone the operations I had done, but I may have been wrong.

I'm getting kind of confused....

This is a very selfish question, but what should I do in this case...?

@CoralPink
Copy link
Contributor Author

I have tried many things,

The cappuccin.css seems to match the one generated via
npm run build and the current one (which I manually rewrote).

Now all I have to do is fix the mistake in package.json?

...Please squash the messy commit history 😖

Copy link
Contributor

@sgoudham sgoudham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to apologise and don't worry about the commit history!

I tested out the PR locally and it looks great! I appreciate you spending the time and effort to contribute to this repo ❤️

@sgoudham sgoudham changed the title fix: Color not applied to diff fix: colours in diff codeblock Oct 2, 2023
@sgoudham sgoudham merged commit a3b3f77 into catppuccin:main Oct 2, 2023
This was referenced Oct 2, 2023
@CoralPink
Copy link
Contributor Author

Thanks for the kind words!

I was actually in a hurry for nothing as I was doing it in the time before bed....

It was a good experience 😅
Thanks again 🩷

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

Successfully merging this pull request may close these issues.

3 participants