Removed feature: implement add to package set from bower #77

justinwoo · 2017-12-21T23:13:21Z

This (mis)feature was never used for managing projects using psc-package. It was only used to do some hacky management of package sets.

fixes #76

> pp install choco-pie                  
Package choco-pie does not exist in package set

> pp add-from-bower purescript-choco-pie                 
Successfully wrote choco-pie to package set.

> pp install choco-pie                  
Updating 38 packages...
psc-package.json file was updated

> cp .psc-package/testing/.set/packages.json .           

> gid                                                    
diff --git a/packages.json b/packages.json
index 758cdad..2bcbcf8 100644
--- a/packages.json
+++ b/packages.json
@@ -228,6 +228,16 @@
     "repo": "https://github.com/purescript/purescript-catenable-lists.git",
     "version": "v4.0.0"
   },
+  "choco-pie": {
+    "dependencies": [
+      "behaviors",
+      "prelude",
+      "record",
+      "typelevel-prelude"
+    ],
+    "repo": "git://github.com/justinwoo/purescript-chocopie.git",
+    "version": "v1.0.0"
+  },
   "codec": {
     "dependencies": [
       "transformers",
@@ -2090,4 +2100,4 @@
     "repo": "https://github.com/paf31/purescript-yargs.git",
     "version": "v3.1.0"
   }
-}
+}
\ No newline at end of file

> ga . && gcm 'fixed my package set'                      
[> 9ca9598] fixed my package set
 1 file changed, 11 insertions(+), 1 deletion(-)

damncabbage · 2017-12-22T00:29:38Z

app/Main.hs

+
+addFromBower :: String -> IO ()
+addFromBower name = do
+  let bowerProc = inproc "bower" [ "info", T.pack name, "--json", "-l=error" ] empty


I think this is good as a first pass, but that we'd want to either emulate the bits of Bower that we need, or make it more obvious that having bower installed as a dependency is a requirement for this feature.

(My personal preference is doing the emulating; as far as I can tell, that means pulling down the JSON registry blob and then looking through it for the package, but fortunately not much else.)

I don't want to reimplement anything from Bower personally. I doubt parsing the registry would be anywhere near as fast as using a user's existing Bower setup and all.

Could be better documented that you need Bower though, yeah.

The registry is super simple - it's just a case of hitting a URL like http://bower.herokuapp.com/packages/purescript-halogen, the registry doesn't actually contain any of the information about the package though, so it means fetching the bower.json from the location the registry returns after that... which is less simple. 😉

I don't love the idea of depending on bower either, but I understand not wanting to reimplement half of it too, so, just thought I'd throw that info in. I did quite a bit of work around this stuff for my bower-replacement-but-still-using-the-registry project that hasn't seen the light of day yet.

FWIW, the old registry URL is deprecated, so it would be better to use this format: https://registry.bower.io/packages/purescript-halogen

(see https://twitter.com/bower/status/918073147789889536)

Uh yes, I actually meant to use that one, must have copied it from some old code and just didn't look at it 😄

kritzcreek · 2017-12-22T13:51:35Z

The code I linked in the corresponding issue gets all the versions by looking at the tags and then downloads the bower.json file for all of these versions. Parsing that json is dead-simple, so using that code you shouldn't need to rely on bower.

Even still I don't think that should go into psc-package itself, I'd rather put it in a separate binary. Maybe alongside the package-set repo?

justinwoo · 2017-12-22T13:59:36Z

@kritzcreek you did say that it pretty quickly requires a Github auth token? Which seems like more of an annoyance.

I don't see the point of having this be separate from psc-package though. By being in the main executable, you can very easily see that it's a possible option -- otherwise, people will have to search around forever to find out that this exists.

paf31 · 2017-12-22T17:11:50Z

app/Main.hs

+addFromBower :: String -> IO ()
+addFromBower name = do
+  let bowerProc = inproc "bower" [ "info", T.pack name, "--json", "-l=error" ] empty
+  result <- fold <$> shellToIOText bowerProc


Instead of shelling out to bower, you could let the user run the info command and pipe it in here. We already try to use some of the Bower JSON formats in purs publish for example, so I think this approach makes sense here too.

But in the end the user wants what happens here right? I wouldn't like to have to look up how to awkwardly use this, and at best, many users would be copy pasting an example command to their terminal and changing it.

hdgarrood · 2018-01-28T20:27:52Z

I think this approach does make sense actually - what we are doing here is specific to bower, so I think it makes sense to shell out to bower rather than expecting some JSON to be supplied on standard input; it doesn’t really make sense to do what the proposed ‘add-from-bower’ command would do but with some other tool instead of bower. I also think it makes sense to put this code in here rather than in a separate executable, as it doesn’t add any noticeable amount of bloat or anything like that: people would still be able to do everything psc-package does currently with no problems if they didn’t have bower installed. However the alternative, as Justin pointed out, is most likely that people who might be able to benefit from this either have to perform an extra install step, or never even learn that this exists.

hdgarrood · 2018-01-28T23:15:18Z

What kind of error do you get if bower is not installed? If that's sensible then I would say this is good to merge.

justinwoo · 2018-02-08T21:33:52Z

It looks like this:

psc-package: bower: createProcess: runInteractiveProcess: exec: does not exist (No such file or directory)

if anyone has a good idea on how to make this more obvious and easier to work with im all ears

justinwoo · 2018-02-11T16:55:12Z

Should we merge this? Seems like the error message wouldn't be even bad for technical users.

* Updates for 0.11.5 * Use smolder v8.0.0 for now * Use smolder-v7.0.0 * Remove pux * Update packages.json

damncabbage reviewed Dec 22, 2017

View reviewed changes

justinwoo force-pushed the add-from-bower branch from f001958 to 7be8342 Compare December 22, 2017 14:06

paf31 reviewed Dec 22, 2017

View reviewed changes

hdgarrood approved these changes Feb 11, 2018

View reviewed changes

implement add-from-bower

18af423

justinwoo force-pushed the add-from-bower branch from 7be8342 to 18af423 Compare February 26, 2018 14:22

justinwoo merged commit 4f9c1ee into purescript:master Feb 26, 2018

Rembane pushed a commit to Rembane/psc-package that referenced this pull request May 26, 2018

Updates for 0.11.5 (purescript#77)

6e62e98

* Updates for 0.11.5 * Use smolder v8.0.0 for now * Use smolder-v7.0.0 * Remove pux * Update packages.json

justinwoo changed the title ~~implement add-from-bower~~ Removed feature: implement add to package set from bower Nov 15, 2018

Removed feature: implement add to package set from bower #77

Removed feature: implement add to package set from bower #77

Uh oh!

Conversation

justinwoo commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This (mis)feature was never used for managing projects using psc-package. It was only used to do some hacky management of package sets.

Uh oh!

damncabbage Dec 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinwoo Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

garyb Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

paulyoung Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

garyb Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

kritzcreek commented Dec 22, 2017

Uh oh!

justinwoo commented Dec 22, 2017

Uh oh!

paf31 Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

justinwoo Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

hdgarrood commented Jan 28, 2018

Uh oh!

hdgarrood commented Jan 28, 2018

Uh oh!

justinwoo commented Feb 8, 2018

Uh oh!

justinwoo commented Feb 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

justinwoo commented Dec 21, 2017 •

edited

Loading

damncabbage Dec 22, 2017 •

edited

Loading