Accept an Option<&str> as a link name #14

Ghost · 2020-11-30T07:04:27Z

Ghost commented

2020-11-30 07:04:27 +00:00

First-time contributor

Many users may be adding links to their documents with &'static str's as the name, which means that they would manually have to convert to an Option, leading to code that looks like link("gemini://emii.gay", Some(String::from("My Website"))) instead of the simpler link("gemini://emii.gay/", Some("My Website")).

We can change this method to accept the latter in addition to accepting Option<String>s, which might make users lives a little easier

Many users may be adding links to their documents with &'static str's as the name, which means that they would manually have to convert to an Option<String>, leading to code that looks like `link("gemini://emii.gay", Some(String::from("My Website")))` instead of the simpler `link("gemini://emii.gay/", Some("My Website"))`. We can change this method to accept the latter in addition to accepting `Option<String>`s, which might make users lives a little easier

Ghost commented

2020-11-30 07:10:59 +00:00

First-time contributor

By the way, if your wondering what I'm doing here and why I'm suddenly baraging you with PRs (sorry i can slow down if yo uneed me too), I'm a dev for the gemini server library northstar and my WIP feature heavy fork kochab, both of which rely on an in-house document building struct.

I wanna see if I can convince panic, northstar's main dev, to adopt gemtext as a replacement, and I'm currently working on that PR and reconciling gemtext with our current Document system. The reason I'm bombarding you with PRs is in an effort to try to smooth out some of the areas where that conversion gets a little messy/less readable, and hopefully benefit others who are using the library as a document builder.

Sorry again for showing up uninvited and all that, and I really won't be offended if you tell me to shove off, but hopefully you find these helpful and not annoying.

Best,
Emi <3

By the way, if your wondering what I'm doing here and why I'm suddenly baraging you with PRs (sorry i can slow down if yo uneed me too), I'm a dev for the gemini server library [northstar](https://github.com/panicbit/northstar) and my WIP feature heavy fork [kochab](https://github.com/Alch-Emi/kochab), both of which rely on an in-house document building struct. I wanna see if I can convince panic, northstar's main dev, to adopt gemtext as a replacement, and I'm currently working on that PR and reconciling gemtext with our current `Document` system. The reason I'm bombarding you with PRs is in an effort to try to smooth out some of the areas where that conversion gets a little messy/less readable, and hopefully benefit others who are using the library as a document builder. Sorry again for showing up uninvited and all that, and I really won't be offended if you tell me to shove off, but hopefully you find these helpful and not annoying. Best, Emi <3

Ghost commented

2020-12-07 16:36:35 +00:00

First-time contributor

Also, I thought this wasn't a breaking change, but I was wrong :(

When passing a None, this needs to be adapted to a Option::<String>::None or Rust is unable to infer the type. As far as possible options, we could:

Require that the name be a String and leave the to as an AsRef
Use a seperate method for adding named links
Just drop the PR
Just require users to do the Option::<String>::None thing

I'm personally in favor of the second option, but the call is yours

Also, I thought this wasn't a breaking change, but I was wrong :( When passing a `None`, this needs to be adapted to a `Option::<String>::None` or Rust is unable to infer the type. As far as possible options, we could: * Require that the name be a `String` and leave the `to` as an AsRef * Use a seperate method for adding named links * Just drop the PR * Just require users to do the `Option::<String>::None` thing I'm personally in favor of the second option, but the call is yours

Ghost commented

2020-12-07 22:44:08 +00:00

First-time contributor

It occurs to me that you've probably already made that decision, and that's why this is like this in the first place, and I naively thought that you had just forgotten it instead of that being a deliberate decision.

Sorry for my arrogance, I'll close this PR

It occurs to me that you've probably already made that decision, and that's why this is like this in the first place, and I naively thought that you had just forgotten it instead of that being a deliberate decision. Sorry for my arrogance, I'll close this PR

Ghost closed this pull request

2020-12-07 22:44:08 +00:00

Ghost deleted branch link-name-str

2020-12-10 16:08:01 +00:00

continuous-integration/drone/pr Build encountered an error

Details