Skip to content

Make lowdown optional#9710

Merged
Ericson2314 merged 1 commit into
NixOS:masterfrom
wegank:disable-lowdown
Jan 8, 2024
Merged

Make lowdown optional#9710
Ericson2314 merged 1 commit into
NixOS:masterfrom
wegank:disable-lowdown

Conversation

@wegank

@wegank wegank commented Jan 8, 2024

Copy link
Copy Markdown
Member

Motivation

This PR makes the lowdown library optional. Specifically, when --disable-markdown is passed, renderMarkdownToTerminal becomes trivial, i.e. Markdown is not rendered:

$ nix help

> **Warning** \
> This program is
> [**experimental**](@docroot@/contributing/experimental-features.md#xp-feature-nix-command)
> and its interface is subject to change.

# Name

`nix` - a tool for reproducible and declarative configuration management
...

Context

lowdown currently has no MinGW support (kristapsdz/lowdown#130). While this PR is inspired by the commit with the same name in #4953, the latter hasn't been updated for two years. I think it's good to have a PR with an atomic commit to drive the changes forward, one by one.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@wegank wegank requested a review from edolstra as a code owner January 8, 2024 17:04
@Ericson2314

Ericson2314 commented Jan 8, 2024

Copy link
Copy Markdown
Member

Thanks @wegank! Can you combine with a3d71fb from #8901?

@Ericson2314 Ericson2314 self-assigned this Jan 8, 2024
@wegank

wegank commented Jan 8, 2024

Copy link
Copy Markdown
Member Author

I don't think readline support has regressed since de59973? Like, it's still possible to build Nix on Linux with

  env = {
    EDITLINE_LIBS = "${readline}/lib/libhistory${stdenv.hostPlatform.extensions.sharedLibrary} ${readline}/lib/libreadline${stdenv.hostPlatform.extensions.sharedLibrary}";
    EDITLINE_CFLAGS = "-DREADLINE";
  };

So I didn't try to make editline optional...

@Ericson2314

Ericson2314 commented Jan 8, 2024

Copy link
Copy Markdown
Member

OK fair enough :) ignore readline/editline for now is fine. But the "yes vs auto" I still had in my configure.ac I think is still good?

You should also do a package.nix update so we are passing the new flag; see how the other flags are done at the Nix level. The flag can be default with hostPlatform.isWindows.

Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
@Ericson2314 Ericson2314 enabled auto-merge January 8, 2024 20:04

@Ericson2314 Ericson2314 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@Ericson2314 Ericson2314 merged commit fa2a27d into NixOS:master Jan 8, 2024
@wegank wegank deleted the disable-lowdown branch January 8, 2024 20:16
@nixos-discourse

Copy link
Copy Markdown

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-on-windows/1113/99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants