piotrmurach / tty-spinner

A terminal spinner for tasks that have non-deterministic time frame.
https://ttytoolkit.org
MIT License
427 stars 28 forks source link

Spinner breaks within a method class #30

Closed quirinux closed 6 years ago

quirinux commented 6 years ago

Hello guys, I really weird behavior started happening o spinner after v0.8.0 release, until few days ago I was using v0.7.0, and it's related to how a run block calls an instance method full code here https://gist.github.com/quirinux/6b3136c9f1dc11b085ebb12483aecd6a

But basically if I set a spinner inside a class method and try to call within spinner run an method of that class it raises a undefined local variable or method error, and as I could check it is related to context change, the only way that I found it working was storing the class self reference in a variable and then call it, which is weird too once this variable is not in local scope only, anyways:

class Foo

    def bar
        sleep 3
    end

    def spin_broken
        spinner = TTY::Spinner.new("[:spinner] :title")
        spinner.run do
            puts self, self.class
            spinner.update(title: %{Foo})
            bar 
            spinner.update(title: %{Bar})
            bar 
            spinner.success
        end
    end
end

Calling Foo.new.spin_broken shows what I mean, the gist linked code above has a more complete example

Running on ruby-2.4.1 release

piotrmurach commented 6 years ago

Hi @quirinux

Thanks for using the library!

The run call hasn't changed since v0.5.0. Basically, the block is evaluated in the context of the spinner. If you wish to use external context then you need to be specific about block arguments and pass in the 'spinner':

spinner.run do |sp|
  sp.update(title: %{Foo})
end

Accepting spinner as a first argument in a block is especially handy for creating multispinners:

multi_spinner = TTY::Spinner::Multi.new("[:spinner] top")
multi_spinner.register("[:spinner] one") { |sp| sleep(2); sp.success('yes 2') }
multi_spinner.register("[:spinner] two") { |sp| sleep(3); sp.error('no 2') }
piotrmurach commented 6 years ago

I've changed to stop evaluating spinner block in the current context to help prevent bugs like yours.