Shoes should have refresh method

[ccoupe]

In issues #151 it was suggested to add a refresh method (to slots or maybe everything?)

I have several thoughts on that.

1. I've wanted one since I first learned Shoes Raisins and
2. it's use would hide bugs that need fixing.

I suspect such a method might be useful for custom widgets or things that Shoes doesn't do well so I'm inclined to favor it.

[IanTrudel]

Suggestion: inheritance should be used here. Top level APP refresh would refresh every associated windows, then refresh on windows, then on slots. Something along this line...

[ccoupe]

I think you mean run time descending down a tree of slots, given a beginning slot entry? Not really Class inheritance. Just redraw the lower part of the tree from that node and below.

From what I can tell, at this moment) in canvas.c, shoes_canvas_repaint_all() does just that. It does NOT start from top of the Window. All it it does is call shoes_slot_repaint() after computing sizes.
It should probably be named shoes_canvas_repaint_slot()

In ruby.c, many widgets have a draw() method in their C api and there is no mention in the Manual that it exists and why you shouldn't call it or when you can.

[ccoupe]

It ends up in app.c (instead of native.c ) which calls shoes_native_slot_paint(slot) which appears, at first glance is not the entire window. I think it's worth trying. It's only a few lines of code in the right places. An hour to implement. And more hours to figure out how to write a test that shows it does or doesn't repaint everything.

[ccoupe]

It's actually quite difficult to write a test case to use the experimental refresh_slot method. First - you need something that is failing. For a few hours we have that opportunity. The fix for #151 hasn't been merged in so master yet

Shoes.app do
  slot = flow do
    para 'Click me!'
    click { slot.remove; slot.refresh_slot }
  end
end

It behaves as intended. If this code makes it into Shoes 3.3 it should come with a warning in the manual that the user is using swinging an axe at his layout problems so please file a bug.

[passenger94]

I knew i had seen this problem recently, it was on snapshot test/demo app
we might need to update the manual, would do a PR if needed.
Also maybe a demo to document refresh_slot method.

Shoes.app width: 400, height: 400 do 
    ext = "svg"
    ext = "pdf"
    path = "#{LIB_DIR}/snapshotfile.#{ext}"

    @slot = stack do    
        button("shoot!") do
            r = snapshot :format => ext.to_sym, :filename => path  do
                stroke blue
                strokewidth 4
                fill black
                oval 100, 100, 200
            end
            @slot.refresh_slot        #new
            info r.inspect
            #alert path, title: "snapshot saved to :"  # temporarily commented here for demo purpose
            #Shoes.show_log                            # ditto
            #para "Done" # comment this out and the svg does not show ### not needed anymore
        end
    end
end

[ccoupe]

I remember that! @passenger94. But is it a bug in snapshot or stack?

[passenger94]

well, since the refresh_slot method do it's job as expected and without it it doesn't show the snapshot also you can make the snapshot appear by simply resizing the window i'm quite sure it's a refresh problem ! so a stack "bug"
but i understand your fear that this have the potential to mask another problem
it could be seen as a snapshot bug, if you try with an image block it indeed works without needing to refresh, so should we had a shoes_canvas_repaint_all to snapshot instead?

Edit : tested shoes_canvas_repaint_all in shoes_canvas_snapshot it works for me
you are right, the use/need of this method probably is to be considered as a sign of a "bug"

[IanTrudel]

Guys, how do you come up with those method names such as refresh_slot and set_window_title (see #93)? The problem is that they are redundantly descriptive since their implicit meaning, as far as OO goes, should be clear enough. What I am saying is that if we have to add new methods, let's make them having sense such as refresh and title.

[ccoupe]

There is an open issue about app.set_window_title. There is also reason that it is different from app.title= It's a very subtle distinction but they are not the same - so they have different name. Feel free to contribute patches to shoes.app w/o breaking everything.

I came up with refresh_slots because I think it is hideous (but occasionally useful). I don't think refresh() or repaint() should be a general method that the user can call to redraw whatever there layout error is. If you re-read the entire thread, @backorder you would note that I'm pretty damn iffy about this proposed experimental method. So I gave it an experimental name. I await your patches for refresh() on all widgets and flows in your OO world.

[IanTrudel]

Your rational is somehow understandable. Perhaps you misunderstand my point. In OO, a class or object responding to a message gives the context. So if a Slot or instance of Slot responds to refresh, it does means refresh slot. Otherwise you end up saying _Slot refresh slot_ when you use refresh_slot.

[ccoupe]

app or Shoes.app as we all know, is a freaking module and adding methods to modules is not OO. 2.x Refinements just make things worse (IMO) because for us it's 'C' down there. I'm a little cranky because I'm going to have to build a couple of new OracleBox VMs for OSX 10.9 and build the deps from source and it's not pretty or fast.

There is nothing OO in Shoes other than widget styles and that isn't consistent. Read the manual again. There may be common names to confuse the innocent but damn little (nothing?) inherits at the C level. Shoes is functional. It only appears to be OO. Shoes is not Ruby ... yada yada.

[IanTrudel]

Shoes is not Ruby is a good reminder. Though Ruby is still in there. In any case, what I said is what makes sense. Shoes has its design issues and we are left with the kid. I am not the one making the final decision and my contributions of late are next to nothing. It is simply my feedback.

Good luck with your builds.

[ccoupe]

Shoes 4 is everything a Ruby purist and modern methods ruby-ist could hope for. They swear on their hipster hats and pinky rings that someone could write a qt or gtk back end that could do most of what is done in Shoes 3.2+. When I run out of things to do with 3.3 I might test that claim. I could also grow old and not care.

[passenger94]

Isn't this already done ?
Edit: Yes it is ! Rename it ? refresh_canvas or refresh ?

[ccoupe]

I was only assigning names and milestones to issues so they can be referenced from the changelog. Current name is fine with me.

[IanTrudel]

This feature is fully implemented under the method name refresh_slot but after some time as passed, I'm still suggesting it becomes simply refresh since its context is implied.

[ccoupe]

Since refresh_slot is part of our legacy (3.3.0) the only backwards compatible option would be to have both methods. Both are fragile crutches indicating a deeper problem in Shoes. Watch samples/good-plot.rb on Linux. - hint - it doesn't work.

[IanTrudel]

How about rename it altogether, change it in the manual and use alias/alias_method on refresh_slot until eventually got rid of it. This method has most likely few users to none. What do you say?

[ccoupe]

Actually, the only thing known to use it at the Ruby level and and need it is the video widget and that's a bit fragile on Windows and older OSX. for different reasons. At the moment, my preference would be to hide either name.

[IanTrudel]

Then it should just be renamed without concerns for backward compatibility.

On a side note, I did use it on #285#issuecomment-259602723

[ccoupe]

I've added method 'refresh' to canvas - both call C shoes_canvas_refresh_slot. Neither appears to work on 3.3.2 for some widgets. See bugs/bug152.rb

[passenger94]

bug152.rb is working as expected !!!
refresh_slot or refresh is only forcing a drawing, nothing else, so if you want to remove evidences you have to cleanup before, then redraw !!! That method exist to force Shoes to do an immediate redraw after changing/removing/adding/ some ui element(s), because of the asynchronous nature of Shoes drawing that could happen with some delay, this method gives instant visual feedback, where otherwise you might have to wait for the next redraw. (note that internally in C, the same method (shoes_canvas_repaint_all) is called very often, almost after every ui changes)

[ccoupe]

Yes. bug152.rb has it's own bugs. Doh!

[ccoupe]

Closed in Shoes 3.3.2