Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Use SwiftPM Resources rather than downloading CSS files. #267

Closed
wants to merge 1 commit into from

Conversation

karwa
Copy link

@karwa karwa commented Apr 29, 2021

This enables offline generation, and allows forks to use their own stylesheets more easily.

@MaxDesiatov
Copy link
Contributor

I remember @mattt previously worked on this, but I'm not sure how that ended. I wonder if there were issues with Linux support, or something of that nature?

@MaxDesiatov MaxDesiatov requested a review from mattt May 3, 2021 17:05
@mattt
Copy link
Contributor

mattt commented May 4, 2021

@MaxDesiatov Yeah, that's right. See #192.

I was excited to switch to package resources. Unfortunately, that feature didn't behave exactly as I expected. For whatever reason, I expected it to work more like Rust's include_bytes!, where the resource was bundled together as part of the built module. But it's more like reading a file from a relative path.

Everything worked when I was testing locally, but it broke when I moved things around during installation. I only learned about this after users reported problems in beta 5, and I had to scramble at the last minute to fix things. I'm still a bit scarred by the experience.

Given the portability challenges for a tool that aims to support macOS, Linux, and Windows, I'm hesitant to introduce any additional complexity around locating files — especially for CSS and JS, which could easily be embedded in source code with a prebuild step. The downside there is that folks couldn't directly edit their installations' resource files to tweak their CSS (on the other hand, that's one less way for things to break).

So that's where I'm at. What do y'all think? Any strong feelings one way or another?

@Lukas-Stuehrk
Copy link
Member

I'd say that all of us agree that we need to ship the assets with the tool itself. This is the right thing to do and it solves a lot of problems.

Given the portability challenges for a tool that aims to support macOS, Linux, and Windows, I'm hesitant to introduce any additional complexity around locating files — especially for CSS and JS, which could easily be embedded in source code with a prebuild step.

When you edit the CSS files, you always need to run postcss in node to generate the minified version. So I think it will be rather easy to extend this step and include the generation of a Swift file which contains the contents of the CSS and JS as strings. Editing the CSS would not require any additional step then. And the problem of making it easy to test the CSS while working on swift-doc is solved.

The downside there is that folks couldn't directly edit their installations' resource files to tweak their CSS (on the other hand, that's one less way for things to break).

I'm not sure if I fully understand. The bundle will contain the minified CSS file, so it's rather hard to actually modify the CSS in there anyway.

So that's where I'm at. What do y'all think? Any strong feelings one way or another?

I don't have any strong feelings or opinions in either direction. Using bundles is the Swift way™ of doing it. But having a binary and a .bundle next to each other without any further packaging also feels extremely weird and uncommon right now, especially for a command-line tool. So I have a slight preference for simply stringify the assets in the Swift source, but really not strong enough to push for any direction.

@mattt
Copy link
Contributor

mattt commented Jun 1, 2021

Thanks for taking a look at this, @karwa. I just merged #288, which takes a slightly different approach.

@mattt mattt closed this Jun 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants