search

Canop / clima

A minimal viewer for Termimad
MIT License
30 stars 7 forks source link

Feature: `target` supports internet addresses for markdown reading #9

Closed dmgolembiowski closed 2 years ago

dmgolembiowski commented 2 years ago

Adds support for passing a URL to clima and downloading it locally to view.

Installation

# cargo install --features web clima
cargo install --features web --path .
Canop commented 2 years ago

I'm OK with the feature but please change the implementation to avoid writing a temporary file. A solution could be to change the arguments given to the viewer: instead of having just a path, it could be a location string and a markdown string.

dmgolembiowski commented 2 years ago

I'm OK with the feature but please change the implementation to avoid writing a temporary file. A solution could be to change the arguments given to the viewer: instead of having just a path, it could be a location string and a markdown string.

Agreed; this would make for a more coherent design. My only gripe is how I currently view the routes which implementation could take. I think splitting these is the right move, and so I'd be inclined to represent this as a clap::ArgEnum. But at the current version clima pins the clap dependency, it would put me in between either: (1) using the outdated arg_enum macro, and thus make any future attempts at updating clima to clap a more recent edition harder, whereas (2) would involve bumping up clap to a newer edition that doesn't use the macro arg_enum, but other parts might need to change (i.e. things not directly related to the location argument).

Is this a valid concern?

dmgolembiowski commented 2 years ago

Closing this for now.

Canop commented 2 years ago

I have no problem in replacing clap, it's too big anyway for such a small program. But I don't think the arg is really a problem: we're given a path, we have to first check whether it's a file and, only if it isn't a local file, check whether it looks like an HTTP URL and then try fetch the file.

If you don't intend on making a PR, I'll probably code the feature at some point.

Canop commented 2 years ago

I just added the feature. You'd be welcome to review it.