shards add/get

[asterite]

I'd like to revive this but under a different issue than #81 (which I don't know if it's exactly about this)

Today I wanted to install this shard in a project (well, actually, I just wanted to try it out). It was straight-forward, I:

• opened shard.yml
• added a dependencies: entry (because I didn't have dependencies yet)
• added gshards:
• added github: bcardiff/ghshard
• saved and ran shards install

That's OK. However, the more we can automate these trivial tasks I think it's better.

Now, we don't have a centralized repository so we can't do shards get some_shard because shards wouldn't know where to find it. But we could have:

$ shards get https://github.com/bcardiff/ghshard

This will find a Resolver (github, gitlab, bitbucket, path) that understands the argument. For github, gitlab and bitbucket we can use a regex, for "path" we can use Dir.dir?(path).

Let's consider the case of GitHub. Once the command find out it's about a github repository, it opens a connection to it and reads its shard.yml to find out its name. Then it adds the name and all necessary info to the existing shard.yml.

So with just a single command we can install a shard. And since we usually see a project in GitHub, copying and pasting that URL into the command line, instead of having to add a few formatted lines into shard.yml, seems easier and less tedious.

The original reason why I discarded this idea is that editing a YAML file is hard: formatting and comments have to be preserved, etc. However, using a pull parser we can more or less know where the "dependencies" section ends and add right after that, assuming 2 spaces of indentation.

I made a program that does that for GitHub here. I wanted to start adding this to shards, taking into account different resolvers, but I first wanted to check with you, @ysbaddaden , if you agree with this. I'm not super strongly in favor of this, so if you say no then it's not that terrible :-)

[kostya]

also #75 about pain with manual edit

[ysbaddaden]

Interesting hack. It assumes a 2 space indentation and that the {} notation isn't used (which is a strong assumption); maybe it should eventually validate the generated YAML file, to verify that it's still valid before saving it?

I'm not sure I really want to have to maintain this feature, but since there is popular demand for it, if we have a good solution for updating shard.yml, why not?

[kostya]

from_yaml -> edit -> to_yaml?

[samueleaton]

I was thinking about this the other day and thought of an add command that's will check to see if a shard exists and add it to the shard.yml

E.g.

shards add kemal --github sdogruyol/kemal --branch master

It would only add it to the shard.yml. You will still need to run shards install to actually install it into the project. This way any project can put the one liner on their README for how to install and it wouldn't be a huge overhaul to implement.

[bcardiff]

@kostya that might change the indentation and blanks a lot.

Isn't the yaml parser capable of tell the location of the different nodes so the edit can be expressed in a controlled diff/patch?

[RX14]

I would definitely call it add instead of get. I think the concept of adding the dependency to shards.yml is the major one here, not getting it from github.

Updating a yaml file while keeping user formatting and comments is hard. I'm not sure if libyaml can do it, and even then if the crystal bindings can do it. I would postpone this feature to work on a nice yaml implementation that can do this.

[asterite]

Yup, my solution is a bit hacky 😊 . However, it's inspired by some rails helpers/generators that add some code to existing Ruby files, for example routes. I remember the devise gem adding a routes entry on installation. I'm pretty sure they don't parse that file, change it and then write it back, preserving formatting and such. I guess they simply try to add it after the line that looks like ...routes.draw do. And I never had a problem with that.

In fact, I just tried devise with a new project. If the file has this:

Rails.application.routes.draw do
end

then it will correctly add devise_for :users after the do. If I change it to:

Rails.application.routes.draw {
}

then it doesn't work anymore (but probably should). This is a minor change, but probably nobody had an issue with that because nobody feels the need to change that.

My point is, this solution is not perfect, but it will work 99% of the time, probably 100% of the time because shard.yml will be generated by either crystal init or shards init, so it will already be formatted the way this command expects, and we usually try to keep it simple and readable. I doubt there will be many projects where shard.yml will look like a JSON. And if that happens, well, the tool will probably say "Sorry, I can't find a way to add it" and that's it.

@ysbaddaden If you don't want to maintain this feature it's fine, I can try to send an initial PR and later take care of it :-)

[ysbaddaden]

It should work almost all the time, so it's acceptable. It just needs to verify that the generated YAML is valid, and it's fine. As long as someone commits to maintain the feature, then I'm OK with it!

I'd call it shards get [uri] or shards add [uri], and maybe hide the shard.yml edit behind a --save flag... or not, I don't know.

[RX14]

I don't think there's any need for downloading a dependency but not adding it to the shards.yml.

[bcardiff]

I like

$ crystal|shards add NAME
$ crystal|shards add NAME --dev

To modify shards.yml. --dev to be a development dependency.
I wouldn't run the installer right away.

NAME should be a description with optional version requirement. And some heuristic to match if git github path etc.

[samueleaton]

@bcardiff Would you want everything to fit into a single NAME argument or would you split it up? What do you think about what I said earlier about flags like --branch, --commit, etc.

[bcardiff]

I don't think we need to cover all possible requirements. github:bcardiff/crystal-foo@0.2.0 Or something like that would be enough IMO. branchs could be covered here after @. I wouldn't mind about explicit commit. Even using just user/repo should match, if exists, a github repo IMO. But i don't mind somo other course of action, just my 2 cents here.

[ysbaddaden]

@samueleaton if typing the command takes as long as editing shard.yml then does it make sense to implement the command in the first place?

[asterite]

I think everyone has a different idea of how this command should work, and all these ideas are different than my proposal 😊

My proposal is:

shards add URL

You pass a URL, and nothing else (no options allowed). From that URL you can quickly tell, using a regex, whether it's github, gitlab, bitbucket, or a filesystem path. It always adds the version found in that URL (probably not specifying any version has the same effect). In my mind it would automatically run shards install after adding the dependency to shard.yml.

Whenever I develop a project, these are the steps I follow:

• I need a functionality, it's not trivial so I think "maybe there's a shard for it"
• I google it and find a couple of projects that might fit my use case
• I check their README to see if they are good and how to use them
• Once I choose one, I want to add it as a dependency

At that point, I'm already at their repository URL, for example their github repository. So all I need to do is copy that URL and type shards add URL.

(I could also copy the code snippet with the YAML bit I need, but it results in a few more steps than the above: find the snippet and copy it (slower than ctrl+l, ctrl+c), open shard.yml, find the correct place to paste it, save the file, go to the command line and write shards install)

I never imagined this command with the idea of specifying a branch or other attributes, because once you need that, writing all those options is the same, or maybe worse, than typing all of that in shard.yml.

[ysbaddaden]

@aterite strong 👍 from me! Go for it.

[samueleaton]

@ysbaddaden It would allow for people to put one-liners in their README for others to copy and paste. But @asterite's solution is good.

[crisward]

@asterite the only addition may be to include the version. Npm does this with the @ symbol

so

crystal add https://github.com/kemalcr/kemal@0.17.4

No version gets latest. Prefer the crystal command to shards, as we're already doing crystal deps?

[RX14]

@crisward If you add it as shards add you could use it as crystal deps add.

I see a lot of people not specifying shard versions because the default generated readme omits any version specifier at all. I'm not sure what the default behaviour in this case is though. For this reason i'm for any way to make specifying versions when installing shards using shards add easier.

[crisward]

crystal deps add https://github.com/kemalcr/kemal@0.17.4

Seems pretty nice to me. I realise install is already there, but if add became install it would mimic npm even more, which could be good for developers migrating from node like me.

crystal deps install https://github.com/kemalcr/kemal@0.17.4

[ysbaddaden]

No. install is for reproducible dependency installations across computers.

[crisward]

Ok. Thought it just did the same as crystal deps. But if that's what it does, make sense to use add. Thanks for the clarification.

[pta2002]

I like @asterite's idea, but I think also adding a --dev flag would be useful. It would function like NPM's --dev flag, adding the dependency as a development dependency instead.

[nedpals]

Hi! I made my own way of dealing this thing and currently implementing it in my own program called("Sharn") so here how mine goes:

$ shards add shardname:username/repo:gitlab@0.1.0/branch

shardname = your shard name
username/repo = repo
gitlab = git platform (when none specified, it automatically defaults to github)
@0.1.0 = specified version
/branch = specified branch

feel free to criticize or whatever. but for me this is somewhat easy and comfortable compared to previous approach.

Sent from my Xiaomi Redmi Note 4 using FastHub

[RX14]

What needs to be done to merge this PR? Is the only issue that it's hacky and may clobber the shards.yml?

[mverzilli]

Sorry Chris, I'm a bit confused: what PR?

[RX14]

@mverzilli oh sorry, I remembered this issue as a PR! Probably because of the attached code sample in the OP.

[ghost]

I like the idea of a simple shards add

• going into shards.yml later and specifying version is easy if required and latest seems like a good default for again 99% of the cases

but if you have a clean shard.yml it is convenient to add the first deps using shards add because it takes care of punctuation and indentation

works fine 99% of the time is the right assumption IMO (like YAGNI)

[faustinoaq]

Hey we should take a look to https://github.com/nedpals/sharn 👍

[ysbaddaden]

Great, someone rolled up their sleeve! I can close this issue then 😁

[RX14]

I disagree that users should need to download an external program or helper for this feature.

[Sija]

@RX14 I agree. Especially since linked shard is not a quality-code we'd like to troubleshoot/debug in case of problems.

[ysbaddaden]

My problem is still the same: I said no to the feature. I won't help implement it and I don't want to maintain it afterwards.

The initial issue proposed something basic: look up an URL create a dependency for it, without reformating the YAML (unlike Sharn). With a big limitation: it only works with a github HTTP URI. Give it anything else and it won't work (bad).

People then came it to ask more features. Being capable to set many CLI arguments to specify versions, refs, and whatever else can be thought to be added —ending up in a feature request that makes it harder and longer and fully duplicates the current way.

For all these reasons, and since nobody proposed any actual and proper solution for almost a year, I'm closing this issue.

[RX14]

The problem to implementing it is the same: we can't roundtrip arbitrary YAML without ruining it's structure. Once that's implemented in whatever YAML library, the feature becomes very few lines of code. I don't see there being much if any maintenance cost.

Of course, round-tripping YAML with comments etc. is a pretty huge feature of the YAML library. The maintenance cost of that would likely be high.

[asterite]

Let's merge sharn in shards. Most people don't have comments in their shards.yaml and standard YAML formatting is spaces, so it should be OK. If shards add breaks your shards.yaml (removes comments, formats it in another way, etc.) and you don't like it, you can simply ignore that feature. For the rest of the world it can be a very useful feature.

[asterite]

Since @nedpals wrote sharn and is maintaining it, I guess he could keep maintaining it here... or otherwise we as a community can do it too.

[donovanglover]

If this feature gets implemented, I suggest having a complementary shards remove to remove installed shards. sharn already has a sharn rm, which is nice.

[faustinoaq]

Another use-case here: https://stackoverflow.com/questions/49395880/central-repository-for-crystal-shards

[nedpals]

Sorry if I disappoint you guys. I will be discontinuing it.

Details:
#192 (comment)

[straight-shoota]

As explained in #144 (comment), the major obstacle for implementing this feature is non-destructive editing of YAML files. If we have a comment in a shard.yml, we mustn't lose that comment. Whitespace and other formatting differences are another, less critical problem.

libyaml (which serves as basis for stdlib's YAML implementation) does not emit comments. They're simply skipped over when parsing a YAML document.

I had an idea how to overcome this anyway: libyaml tracks the location of parser events. We can use that to determine the location where to insert (or remove) YAML code in the file content.

Take this YAML document for example:

name: foo

dependencies:
  bar:
    git: https://example.com/bar/bar
dev_dependencies:
  baz:
    git: https://example.com/baz/baz

Assuming we want to insert a key value pair at the end of the mapping at dependencies.
The parser consumes the YAML document and we ignore everything until we reach the target path dependencies (at 3:0). Then we consume its value (still ignoring everything) until we reach its end (which is a MAPPING_END event at 6:0).
This gives us the insert location. At this point, we can abandon the YAML parser and work on the plain text string data.
I've given the location as line:column, but the string index is also available. So we can simply split the text at that location, insert the additions, and join all three parts again. Applying proper indentation to the inserted code shouldn't be a problem.

Remove or update operations can work in a similar manner.

[vlazar]

As explained in #144 (comment), the major obstacle for implementing this feature is non-destructive editing of YAML files. If we have a comment in a shard.yml, we mustn't lose that comment. Whitespace and other formatting differences are another, less critical problem.

libyaml (which serves as basis for stdlib's YAML implementation) does not emit comments. They're simply skipped over when parsing a YAML document.

Doesn't seem like support for comments or other features is coming to libyaml any time soon if ever.

yaml/libyaml#20 (comment)
yaml/libyaml#269 (comment)

They recommend trying libfyaml instead.

Unfortunately:

Missing features and omissions

• Windows - libfyaml is not supporting windows yet.
• Unicode - libfyaml only supports UTF8 and has no support for wide character input.

[straight-shoota]

The shard.yml schema is pretty trivial and straightforward. I'm sure a completely dumb, string-search based insertion would work just fine. It would be nice to to this with full understanding of the YAML syntax, but I don't think it's a necessity to get a working tool.

For example, the algorithm could be: Look for a line that starts with dependencies:, then search for the next line that has a word character at the first column, backtrack empty lines and that's your insert location.