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

utility/preview: added markdown-previewer.nvim and moved glow #217

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

Donnerinoern
Copy link
Contributor

@Donnerinoern Donnerinoern commented Feb 12, 2024

Description

Added markdown-previewer.nvim as a Markdown previewer and moved both it and glow to a new category: utility/preview.

Type of change

  • New feature
  • Breaking change (move of glow may break some configs)

Checklist

  • My code follows the style and contributing guidelines of this project.
  • I ran Alejandra to format my code (nix fmt).
  • I have performed a self-review of my own code and tested it.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • This change requires a documentation update.
  • I have updated the documentation accordingly.

Screenshots & Logs

image

modules/languages/markdown/markdown.nix Outdated Show resolved Hide resolved
modules/languages/markdown/markdown.nix Outdated Show resolved Hide resolved
@NotAShelf
Copy link
Owner

I'm not sure if we really want the previewer in languages/markdown.nix directly. Generally companion plugins are sorted by function and not language, going under !utilty for the most part. This is fine but I'd rather avoid non-LSP plugins in the language modules.

@Donnerinoern
Copy link
Contributor Author

Fair point. I thought about moving it, but didn't because Glow already was there. I'll keep that in mind for future PRs.

I'll make a new commit in a little bit.

@Donnerinoern Donnerinoern force-pushed the language/markdown branch 8 times, most recently from 608fcbf to c2b871e Compare February 13, 2024 12:51
@Donnerinoern
Copy link
Contributor Author

I hope this new commit is satisfactory. If not, let me know, and I'll change it. It moves both glow and markdown-preview to a new preview category in utility. I also fixed types.listOf types.strand mkOption.

@Donnerinoern Donnerinoern changed the title language/markdown: added markdown-previewer.nvim utility/preview: added markdown-previewer.nvim and moved glow Feb 13, 2024
@Donnerinoern
Copy link
Contributor Author

While the IP-address and port options do play nicely with empty strings, they're not specified to be empty strings in upstream, just empty (unlike other options). I therefore added optionalString just in case.

NotAShelf
NotAShelf previously approved these changes Feb 13, 2024
@NotAShelf
Copy link
Owner

NotAShelf commented Feb 13, 2024

LGTM!

requesting a review from @FrothyMarrow, given the relatively large scope of the PR in case I've missed anything

Copy link
Contributor

@FrothyMarrow FrothyMarrow left a comment

Choose a reason for hiding this comment

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

👍

modules/utility/preview/glow/config.nix Outdated Show resolved Hide resolved
modules/utility/preview/glow/config.nix Outdated Show resolved Hide resolved
Copy link
Owner

@NotAShelf NotAShelf left a comment

Choose a reason for hiding this comment

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

LGTM!

@NotAShelf
Copy link
Owner

Merging when the CI passes.

@NotAShelf NotAShelf merged commit d2334ca into NotAShelf:main Feb 14, 2024
7 checks passed
@NotAShelf
Copy link
Owner

Missing documentation, but I'll add it. Thanks!

bloxx12 pushed a commit to bloxx12/nvf that referenced this pull request Sep 29, 2024
utility/preview: added markdown-previewer.nvim and moved glow
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