spadarian / docblock-python

Atom plugin to insert documentation blocks for python functions
GNU General Public License v2.0
22 stars 9 forks source link

The last update 0.17.3 has conflicts with other packages #43

Closed maky-hnou closed 4 years ago

maky-hnou commented 4 years ago

I opened Atom, everything is working as usual. I got a notification that docblock-python needs to be updated, I updated the package and restarted Atom, but I was surprised that minimap does not work anymore!
I uninstalled minimap and reinstalled it again, uninstalled Atom, removed the configs, and installed atom and minimap all along with docblock-python. However minimap still not working.
I uninstalled docblock-python, minimap is showing again!

I think the last update had messed something, please check it.
Thank you.

gilbertohasnofb commented 4 years ago

Yes, I am also conflicts between this package and minimap, the latter stopped working until I disabled this one here.

maky-hnou commented 4 years ago

@gilbertohasnofb I downgraded docblock-python to the version 0.16.0 to keep using both minimap and docblock-python.
I used this command on Ubuntu: apm install docblock-python@0.16.0

gilbertohasnofb commented 4 years ago

Thank you!

spadarian commented 4 years ago

Thanks for your report. I will look into it during the week.

spadarian commented 4 years ago

There seems to be a problem with minimap and, since it's not currently maintained (the last commit was 17 months ago), I'm not sure how we can fix that.

I think it is possible to fix it by changing line 391 from the function initSubscriptions in the file minimap/lib/main.js:

--- a/lib/main.js
+++ b/lib/main.js
@@ -388,7 +388,7 @@ class Main {
   initSubscriptions () {
     this.subscriptions.add(atom.workspace.observeTextEditors((textEditor) => {
       let minimap = this.minimapForEditor(textEditor)
-      let minimapElement = atom.views.getView(minimap)
+      let minimapElement = this.minimapViewProvider(minimap)

       this.emitter.emit('did-create-minimap', minimap)
       minimapElement.attach()

Do you mind trying that to see if it works in your case?

maky-hnou commented 4 years ago

That worked, thank you!
Please report that bug to minimap

UziTech commented 3 years ago

@spadarian why does that fix this issue?

I'm trying to bring mimap back to life and before I merge https://github.com/atom-minimap/minimap/issues/694 I want to know more about the underlying issue.

aminya commented 3 years ago

I haven't faced this issue. We should first understand it before we apply fixes in minimap.

spadarian commented 3 years ago

@UziTech @aminya I don't remember exactly. https://github.com/spadarian/docblock-python/pull/42 added a style selection menu and, when minimap was present, atom.views.getView would select that menu instead of the minimap (if I remember correctly).

Anyway, I ended up reverting that merge because it had conflicts with other packages and I don't have time to look into that at the moment (it is not an crucial part of docblock-python).

UziTech commented 3 years ago

I see the problem, the atom.views.addViewProvider should check for the correct model or it will return an element every time atom.views.getView() is called by any package.

You can see https://github.com/atom-minimap/minimap/blob/fefc23e747a89c1309fff90a88d5b2492a80a0f7/lib/main.js#L128 as an example

UziTech commented 3 years ago
- atom.views.addViewProvider(() => {
-   return new StyleChooserView(state.StyleChooserViewState).element;
+ atom.views.addViewProvider(StyleChooserView, (model) => {
+   return model.element;
  });
aminya commented 3 years ago

I think making a PR is a good idea! @UziTech

UziTech commented 3 years ago

It sounds like it is not needed anyway since it was removed. I just thought I would put this out there in case they wanted to add it again.

spadarian commented 3 years ago

Thanks guys. I notified the author of the PR in case he wants to try again.