Support `thor install <uri>` to install remote thor files by deivid-rodriguez · Pull Request #787 · rails/thor

deivid-rodriguez · 2022-06-03T11:42:35Z

The previous code suggested that this was supported, but it was not really working as expected.

dorner · 2022-06-03T15:44:57Z

Tests seem to be failing. Also, can you please describe what the problem was and how this fixes it?

deivid-rodriguez · 2022-06-05T17:30:34Z

Yes, test are failing in the main branch. #780 should be merged first, and then I can rebase this to get it green hopefully :)

Sorry for not describing this enough.

As the title says, this PR intends to fix running thor install <uri>. The thor CLI provides an install command which receives a Thorfile as an argument. This argument can be a local filename, a directory (in which case Thor looks for a main.thor file inside) or a remote URI. However, passing a remote URI does not work at the moment and Thor raises an error like the following:

$ thor install https://raw.githubusercontent.com/rails/thor/main/Thorfile
Error opening file 'https://raw.githubusercontent.com/rails/thor/main/Thorfile'

To fix the problem, I detect whether the given argument is an uri or not. If it's an uri, I use URI.open, otherwise I use open like it's done now.

Let me know if there's more I should clarify!

dorner · 2022-06-07T14:16:36Z

@deivid-rodriguez I think the issue has to do with the changing of the functionality of open in Ruby 3.0. We'd need to make sure that older Rubies are still supported. https://stackoverflow.com/a/66032553/5199431

deivid-rodriguez · 2022-06-07T14:30:44Z

You're right, it sounds like this is broken since Ruby 3.0. And my new approach only works for rubies where URI.open is public.

I actually based this solution on what's done at

thor/lib/thor/actions.rb

Lines 204 to 234 in ab3b5be

    
           # Loads an external file and execute it in the instance binding. 
        
           # 
        
           # ==== Parameters 
        
           # path<String>:: The path to the file to execute. Can be a web address or 
        
           #                a relative path from the source root. 
        
           # 
        
           # ==== Examples 
        
           # 
        
           #   apply "http://gist.github.com/103208" 
        
           # 
        
           #   apply "recipes/jquery.rb" 
        
           # 
        
           def apply(path, config = {}) 
        
             verbose = config.fetch(:verbose, true) 
        
             is_uri  = path =~ %r{^https?\://} 
        
             path    = find_in_source_paths(path) unless is_uri 
        
             say_status :apply, path, verbose 
        
             shell.padding += 1 if verbose 
        
             contents = if is_uri 
        
               require "open-uri" 
        
               URI.open(path, "Accept" => "application/x-thor-template", &:read) 
        
             else 
        
               open(path, &:read) 
        
             end 
        
             instance_eval(contents, path) 
        
             shell.padding -= 1 if verbose 
        
           end

, so it sounds like that's already broken like in this PR.

Anyways, I will amend this to support old rubies once the repo gets some activity and CI in the main branch is back to green 👍.

deivid-rodriguez · 2022-06-17T08:18:46Z

@dorner This should be green now. As noticed, there's some code in thor/lib/thor/actions.rb where open-uri usage is also broken. But I'd rather investigate that on a separate PR.

dorner

Looks good to me. @rafaelfranca ?

deivid-rodriguez · 2022-06-23T11:52:23Z

For what it's worth, my initial motivation to change this was to avoid using

class Thor::Runner < Thor #:nodoc:
   autoload :OpenURI, "open-uri"
   # ...
end

because that just got deprecated and will no longer work as it works today in the future. See ruby/ruby#5949.

But then when I attempted to change the code to not use the above, I realized that the code was not actually working, so I went ahead and fixed that too.

The previous code suggested that this was supported, but it was not really working as expected.

deivid-rodriguez · 2022-06-27T19:48:50Z

Rebased!

deivid-rodriguez · 2022-07-03T13:32:44Z

By the way, I think this PR is maybe a bit complicated to review due to indentation changes, I recommend looking at the patch with w=1: https://github.com/rails/thor/pull/787/files?w=1.

deivid-rodriguez · 2023-02-08T20:16:17Z

Thanks so much @rafaelfranca!

deivid-rodriguez mentioned this pull request Jun 3, 2022

Fix specs to work with latest rspec-mock #788

Closed

deivid-rodriguez force-pushed the support-uris branch 2 times, most recently from 3684eae to 59ad393 Compare June 17, 2022 08:17

dorner reviewed Jun 17, 2022

View reviewed changes

Support thor install <uri> to install remote thor files

439a593

The previous code suggested that this was supported, but it was not really working as expected.

deivid-rodriguez force-pushed the support-uris branch from 59ad393 to 439a593 Compare June 27, 2022 19:48

deivid-rodriguez mentioned this pull request Dec 13, 2022

Prefer to use URI.open and File.open instead of Kernel.open #802

Merged

rafaelfranca merged commit 0e40ffa into rails:main Feb 8, 2023

deivid-rodriguez deleted the support-uris branch February 8, 2023 20:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `thor install <uri>` to install remote thor files#787

Support `thor install <uri>` to install remote thor files#787
rafaelfranca merged 1 commit into
rails:mainfrom
deivid-rodriguez:support-uris

deivid-rodriguez commented Jun 3, 2022

Uh oh!

dorner commented Jun 3, 2022

Uh oh!

deivid-rodriguez commented Jun 5, 2022 •

edited

Loading

Uh oh!

dorner commented Jun 7, 2022

Uh oh!

deivid-rodriguez commented Jun 7, 2022

Uh oh!

deivid-rodriguez commented Jun 17, 2022

Uh oh!

dorner left a comment

Uh oh!

deivid-rodriguez commented Jun 23, 2022 •

edited

Loading

Uh oh!

deivid-rodriguez commented Jun 27, 2022

Uh oh!

deivid-rodriguez commented Jul 3, 2022

Uh oh!

deivid-rodriguez commented Feb 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

deivid-rodriguez commented Jun 3, 2022

Uh oh!

dorner commented Jun 3, 2022

Uh oh!

deivid-rodriguez commented Jun 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dorner commented Jun 7, 2022

Uh oh!

deivid-rodriguez commented Jun 7, 2022

Uh oh!

deivid-rodriguez commented Jun 17, 2022

Uh oh!

dorner left a comment

Choose a reason for hiding this comment

Uh oh!

deivid-rodriguez commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deivid-rodriguez commented Jun 27, 2022

Uh oh!

deivid-rodriguez commented Jul 3, 2022

Uh oh!

deivid-rodriguez commented Feb 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

deivid-rodriguez commented Jun 5, 2022 •

edited

Loading

deivid-rodriguez commented Jun 23, 2022 •

edited

Loading