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 definition lists #117

Merged
merged 12 commits into from
Feb 3, 2025
Merged

fix definition lists #117

merged 12 commits into from
Feb 3, 2025

Conversation

TerminalFi
Copy link
Contributor

@TerminalFi TerminalFi commented Jan 30, 2025

Config was formatted incorrectly. Bumped vitepress at the same time

fixes #86

@TerminalFi TerminalFi requested a review from FichteFoll January 30, 2025 02:12
Copy link
Member

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I have a few ideas on how to tackle my comments but I'd like you to answer some of my questions first before I begin diving into it myself.

@FichteFoll
Copy link
Member

I have revised the code here and pushed onto the branch. Looks good from my side now. PTAL.

@TerminalFi
Copy link
Contributor Author

Looks good to me

@@ -19,10 +21,10 @@ export default defineConfig({
title: 'Sublime Text Community Documentation',
description: 'Community-driven Documentation for Sublime Text',
head: customHead,
appearance: 'dark',
Copy link
Contributor

Choose a reason for hiding this comment

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

just in case you didn't know, a local npm install & using LSP-typescript in Sublime helps to prevent a wrong config

Copy link
Contributor Author

@TerminalFi TerminalFi Feb 2, 2025

Choose a reason for hiding this comment

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

Unless something is wrong in my setup. These things did not throw an error. I caught it by going through the config by hand.

Copy link
Member

Choose a reason for hiding this comment

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

I almost did that today, but ended up not doing it today because I haven't fully committed my LSP configuration of another machine to my dotfiles repo yet and didn't want to reconfigure everything from scratch.

(I hope LSP-vue works with vitepress …)

Also use the custom <Term> tag throughout the entire documentation texts
because the `:term:text_with_underscores:` syntax is inferior to simply
including the text (with a slot).

With these changes, there is also little to no resemblance to the
vuepress glossary plugin that was used before, meaning we should not
violate any license anymore.
I initially only committed this because I worked on them anyway, but
after I had problems getting it to work in unexpected ways and changed
all occurrences to use the Term tag directly, we don't need this
(and the dependency) anymore.
Using the default (`true`) means that the user's "preferred color
scheme" will be followed, which I interpret as whether the system theme
is light or dark. This is a sensible default that we don't need to
override
@FichteFoll
Copy link
Member

I noticed I forgot to commit 4 files, so I force-pushed them. And made a few more changes on top. Merging this to deploy it.

@FichteFoll FichteFoll merged commit 8770980 into master Feb 3, 2025
@FichteFoll FichteFoll deleted the fix-86 branch February 3, 2025 00:31
@FichteFoll FichteFoll added bug Something isn't working theme Issue with theme style, format, layout, etc... labels Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working theme Issue with theme style, format, layout, etc...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Definition lists don't work
3 participants