PR: Fix crash on empty project explorer context menu

[timhoffm]

fixes #5594

The fix works, even though, I'd recommend a change in the design. IMHO setup_view() should be part of the __init__, because it's necessary to obtain a fully functional (even if maybe emtpy) widget.

[ccordoba12]

@csabella, please review this one.

[csabella]

because it's necessary to obtain a fully functional (even if maybe emtpy) widget

Hi @timhoffm, thanks for your contribution!

Can you explain why you added the setup_view and why it's necessary to obtain a fully functional widget? What functionality is lost without calling it? Thanks!

Sorry, I accidentally hit the close button.

[timhoffm]

@csabella conerning setup_view(): I had a second issue in that context. However, I cannot reproduce the case anymore. As far as I remember, self.menu was None so that self.menu.clear() raised an exception. Then again, one would have to call setup() and not setup_view(). Maybe I've mixed them up.

IMHO the contract of ExplorerTreeWidget should be more clear. ProjectExplorerWidget uses

self.treewidget = ExplorerTreeWidget(self, self.show_hscrollbar)
self.treewidget.setup(name_filters=self.name_filters, show_all=self.show_all)
self.treewidget.setup_view()

so, __init__(), setup() and setup_view(). Are those all mandatory? Not according to

self.emptywidget = ExplorerTreeWidget(self)

However, the menu gets only instantiated in setup() and the internal update_menu() seems to rely on that.

TLDR: Since I currently cannot reproduce my case, we could leave out setup_view(). However, I'm fearing that there are some issues hidden in the current design of ExplorerTreeWidget with its implicit assumtions.

[csabella]

@timhoffm I believe you receive the error on the menu if the change to get_selected_filenames() returned None instead of [].

I think the point of emptywidget was to not have to instantiate everything that happens in setup_view(). treewidget and emptywidget are both created and added to the same layout and one is always hidden and one is always shown. If the purpose of emptywidget was to be the same as treewidget, but without a tree, then why not just use a treewidget with nothing in it? @ccordoba12 added emptywidget, so maybe he can add more color, but right now I'm leaning towards not adding setup_view() to emptywidget.

[ccordoba12]

If the purpose of emptywidget was to be the same as treewidget, but without a tree, then why not just use a treewidget with nothing in it?

Yep, that was the purpose. I have to admit I added EmptyWidget in a bit of a rush before releasing Spyder 3, but if we can get rid of it I think it'd be better.

@csabella, how could we use TreeWidget to be empty?

[csabella]

@ccordoba12 I don't know if a TreeWidget could be empty, but I was thinking that emptywidget was essentially an empty tree, so that's why I said that. If treewidget can't be toggled between an empty version and a populated version, then emptywidget might be the best solution. Or maybe emptywidget should be created a different way? Instead of being an ExplorerTreeWidget, make a different blank widget? That would make it less confusing perhaps. Or maybe even instead of a blank tree, put something else there, like an empty project that shows the same directory structure as File Explorer? I'm not sure what options that were considered before for this.

In any event, I think the best move might be just to create a new issue to address the emptywidget questions and leave this PR to just fix the context menu issue. Technically, it's caused by emptywidget, but there isn't any harm in adding the check for no model even if emptywidget isn't used.

[csabella]

@timhoffm Please remove the emptywidget.setup_view(). We can open that as a separate issue since there might be additional changes. Thanks!

[ccordoba12]

In any event, I think the best move might be just to create a new issue to address the emptywidget questions

Could you open it, please? Let's keep the discussion in that new issue instead of here.

---

@timhoffm, thanks a lot for your contribution and @csabella for your careful review!