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

Zed #2262

Closed
1 task done
tecandrew opened this issue Feb 7, 2024 · 13 comments · Fixed by #2281
Closed
1 task done

Zed #2262

tecandrew opened this issue Feb 7, 2024 · 13 comments · Fixed by #2281
Labels
waiting for transfer Waiting on the repository being transferred from the maintainer

Comments

@tecandrew
Copy link
Contributor

tecandrew commented Feb 7, 2024

Link to repository

https://github.com/tecandrew/zed.git

Screenshots

image

image

image

image

Any additional comments?

Catppuccin

  • the cursor colors were used for each flavor, but made it hard to read when highlighting the in terminal (highlight color == cursor color), so I adjusted each flavor's cursor color slightly
    • the highlight color in the editor panes was changed to use the same terminal cursor color
  • color fields set to null falls back to Zed default color (not sure what this is), but mostly look OK

Zed

  • most of the coloring was transferred over using Zed's vscode theme importer tool noted in their blog
    - custom color themes currently only available in Zed Preview
  • Zed JSON color scheme defined here

Submission Guidelines

  • I have read and followed Catppuccin's submission guidelines.

edit:

there's a slight issue with the selection highlight color not being used in the terminal, and has been identified as a defect zed-industries/zed#7532

Fixed in v0.123.2-pre. Should be in Stable soon

@uncenter
Copy link
Member

uncenter commented Feb 8, 2024

I can confirm no issues with it so far, looks really good. Nice job @tecandrew!

@viralams
Copy link

viralams commented Feb 9, 2024

This is fantastic. I dont see any issues either. Awesome work! @tecandrew

@kjvdven
Copy link

kjvdven commented Feb 14, 2024

Looks awesome! Thnx for the port.

I also use the VS Code theme, but as flat appearance with minimal set, could you add that as a variation?

Also the colours look a little different, but maybe that is because flat appearance.

Screenshot 2024-02-14 at 10 14 50

Now I hope we can set the catppuccin icons in the future in Zed

@sgoudham
Copy link
Contributor

sgoudham commented Feb 14, 2024

Hey 👋, thanks for working on this!
I've just got a few comments regarding the theme and the codebase:

Theme

  1. Overall it looks good! Looks like the VSCode importer got us most of the way there :D

    In regards to the syntax highlighting, it looks like Zed is using treesitter. Funnily enough, we're still in the process of updating our style guide (docs(style-guide)!: improve and define more highlights #2109) to explicitly define semantic highlights. Its similar enough to VSCode/Intellij/etc which is pretty good, considering Zed hasn't fully finalised its schema.

    That being said, I'm curious why the .js file in @kjvdven's screenshot has every single variable in red because that definitely is incorrect.

  2. Could you revert the background colour of the text on the top bar here:

    image

    I think it'd look much better if it matched the background colour of base

Codebase

  1. I believe the changes to the file tree have been made, I'd appreciate if you could re-generate the catwalk images to reflect that.

  2. Please update the README badges to point towards catppuccin/zed.

  3. Please convert the README asset links to be relative urls.

  4. I think the README Usage section could be simplified to the following:

    1. Create your custom theme folder if you haven't already.
    	```bash
    	mkdir ~/.config/zed/themes/
    	```
    2. Download the [catppuccin.json](./catppuccin.json) file into the theme folder.
    3. Open Zed.
    4. Select your Catppuccin theme in the dropdown shown after hitting ( `cmd+k`, `cmd+t` )
  5. This isn't necessary for the transfer into the organisation, but this repository could benefit from using whiskers, our new work-in-progress port creation helper tool, which is aimed at generating catppuccin ports via handlebars template files. I recently added single-file support which would allow it to be used here. (it's still unreleased so you'd have to install straight from source)

Think we'll be good to transfer the repository into the organisation after the above changes have been implemented!

@sgoudham sgoudham added the waiting on author Waiting on action from the maintainer label Feb 14, 2024
@wonderbeel
Copy link

wonderbeel commented Feb 15, 2024

Can add my 2 cents after using this theme for a couple of days and trying to tweak it:

  • borders are too high contrast (this is personal preference but for example the VS Code theme is not as strong) so I tried to reduce them going from overlay1 and overlay0 to surface1 and surface0
  • there is a weird border around the entire window but it seems to be a generic issue with dark themes (it happens also with built in one dark but not with one light, so I guess that it is just an issue with zed)
  • as already noted ghost items use a color that works well in the title bar and tab bar but not so well in the toolbar: looking at other themes the best solution seems to put this value to transparent

Couple of screenshots:

Screenshot 2024-02-15 alle 09 18 25
(@tecandrew theme)

Screenshot 2024-02-15 alle 09 38 42
(my tweaks)

Screenshot 2024-02-15 alle 09 30 45
Screenshot 2024-02-15 alle 09 30 53
(one dark and one light as a reference)

@kjvdven
Copy link

kjvdven commented Feb 15, 2024

That being said, I'm curious why the .js file in @kjvdven's screenshot has every single variable in red because that definitely is incorrect.

Do you need something from me to research the issue?

@tecandrew
Copy link
Contributor Author

tecandrew commented Feb 19, 2024

@sgoudham Got all but the last one in the repo 👍🏽 never heard of the whiskers tool until now: would have definitely helped a bit when i got started on this! 🫠

other things addressed was update the borders as well: the lower contrast looks does look a bit better. null value was tried too, but looks like Zed defaults to some sort of 'light yellow' color

regarding the Red color, the importer transferred over the "correct" color for the variables, but seems like there's 2 'flavors' for variables defined by zed to set: https://github.com/tecandrew/zed/blob/ac91f746cadd48eef030e0b8be3734bd74e19e7b/catppuccin.json#L289

throughout my testing on some local projects i can't seem to tell the affects on it. their schema doesn't have documentation notes on it either 🙃

@wonderbeel
Copy link

Just an heads up, I was looking at the extensions repo and noticed that there is already a catppuccin theme that seems to be this: it seems to be a theme generated with the importer from VS Code instead of hand tuned but it is already there in the centralized extensions repo 🫠.

@sgoudham sgoudham removed the waiting on author Waiting on action from the maintainer label Feb 19, 2024
@sgoudham
Copy link
Contributor

sgoudham commented Feb 19, 2024

Thanks for making the changes I outlined @tecandrew

In regards to the red colour, the syntax highlighting keys seem to be 1:1 mapping to treesitter so I think we'd want to change our instance of variable to text, as previously defined in catppuccin/nvim. This should stop the entire file being red if there is no LSP defined providing another set of highlights. I'm unsure about variable.special since that's not even defined in nvim-treesitter but makes sense to change that to text as well.

I get the feeling that we'll need to iteratively improve on the syntax highlighting so I'm not too bothered about getting it close to our editors right now, that being said, I've just noticed some things that we can easily line up:

(I'm assuming that Zed will be following the syntax highlights defined in nvim-treesitter)

  1. boolean and constant should be peach.
  2. string.regex, string.escape and string.special should be pink.
  3. string.special.symbol should be flamingo.
  4. tag should be blue.
  5. variable and variable.special should be text. (as discussed above)

For testing, I'd recommend that you check out our samples directory which has code samples for a bunch of popular languages. You could probably have Zed open on one side and VSCode on the other and review them both - making sure that nothing is too out of the ordinary. (Obviously there'll be differences in what the editor recognises as a "variable" or "type" but I think comparing against VSCode is a good sanity check)

Hopefully should be good to transfer into the organisation after these changes since the UI/Syntax Highlighting is coming along nicely! Also thanks for being patient while we go back and forth on this! Really appreciate your (and everyone elses) inputs into making the theme nicer before transferring it ❤️

@sgoudham sgoudham added the waiting on author Waiting on action from the maintainer label Feb 19, 2024
@sgoudham
Copy link
Contributor

Just an heads up, I was looking at the extensions repo and noticed that there is already a catppuccin theme that seems to be this: it seems to be a theme generated with the importer from VS Code instead of hand tuned but it is already there in the centralized extensions repo 🫠.

Yeah I noticed this too, I think it makes sense to try and upstream the theme here once merged and available as catppuccin/zed.

@tecandrew
Copy link
Contributor Author

set the coloring recommendations you mentioned to be similar to treesitter. the sample codes are great! found some discrepancies that the importer didn't get quite right.

few things I noticed from the samples:

  • the classes themeselves (when attributes are used) seem to be Red and this is affected by the variable.special color. so I set it to that instead of text to be similar to vscode. (right is current Zed). classes when functions are used/set are colored differently and left alone to be text color

image

  • the brackets are colored differently in vscode and change the more nested they are. Zed however doesn't seem to do this, so it's set to the Teal color (set by the importer tool)

image

@sgoudham sgoudham removed the waiting on author Waiting on action from the maintainer label Feb 20, 2024
@sgoudham
Copy link
Contributor

Cheers! Thanks for updating it to be in line with VSCode @tecandrew

I think what you've done with the brackets and variables is good given the limitations!

The only change that I'd be looking for now is regenerating the catwalk previews on the main README so that the selection highlight isn't yellow (although I suppose it's only in preview - maybe worth mentioning that preview is recommended given that syntax highlighting is still evolving / being bug-fixed?)

In any case, the catwalk preview can be easily regenerated at any point so I'm happy to see this transferred into the organisation!

Please read, "Transferring a repository owned by your personal account", if you are unsure about the process of transferring the repository to Catppuccin.

We'll close this issue once the port has been added to our ports.yml. You can find an example of this in #2164 (in this case, category would be code_editor)

@sgoudham sgoudham added the waiting for transfer Waiting on the repository being transferred from the maintainer label Feb 20, 2024
@tecandrew
Copy link
Contributor Author

tecandrew commented Feb 20, 2024

no prob! I regenerated the catwalk images, but seemed like GitHub's relative links render thing took a bit to update the README previews. it should be good now 😄

from the Zed discord, staff mentioned they release a new Preview and Stable version every week. so likely the selection bug fix should get to Stable fairly soon

edit:

transferred 🫡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for transfer Waiting on the repository being transferred from the maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants