dash-docs-el / helm-dash

Browse Dash docsets inside emacs
511 stars 59 forks source link

Filters #16

Closed kidd closed 10 years ago

kidd commented 10 years ago

From @agpchil 's code, I did my take on the filtering. Not very clear which is the best approach. As it is now.

We don't have a way to deactivate a docset via elisp. The 'active' docsets are all the docsets installed in the machine. Then the filtering is ment to be done on a per-buffer basis.

If there's no modification of the per-buffer, we use the global one, which has everything.

agpchil commented 10 years ago

Nice! I just tried it and it works pretty well. I think your approach is much better. Being able to set docsets per buffer using add-hook is great :)

However I'm still unsure about removing helm-dash-deactivate-docset. Without this there's no way to interactively disable a docset.

Also, maybe I'm wrong but I think remove-if-not is defined in cl so should we add a (require 'cl) ?

kidd commented 10 years ago

ouch yeah, you're right on the 'cl . will they finally encourage the usage of cl one day...? :)

we should think of how we want/think ppl will use it.

I'll summarize my ideas about the deactivate later... busy now EDIT: if there are the 2 layers (global and local), when removing from global active docsets, they will be removed also from local, but not from the Hard drive, so in fact we have 3 layers. what should happen if the user installs the same docset again? be activated? just redownloaded? Too much context needed by the user IMO.

Then, we are aiming for giving a sane and automated workflow to the user, pretending the layers do not exist or are transparent, and giving them a limited API,

I think if the user is smart enough to use helm-dash (using hooks depending on major-mode), we can tell him that, if he wants to install something, use helm-install-....... , to remove something, use rm ~/.docsets , and the buffer-local settings will do the rest. we won't need defcustom anymore, making it simpler, lowering the bar for newcommers.

The 'global' setting will be doing an 'ls' and getting all the docsets you have there.

Does it make sense? @areina, what's your take?

agpchil commented 10 years ago

The user will need to rm ~/.docsets/foo to deactivate a docset globaly? and to activate again do a helm-install-... ? I see too much reinstalls here... but this is somehow similar how package-list-packages work...

The global setting will scan installed docsets when helm-dash is invoked or when the library is loaded? Also, global will be used only to setup the docset connections?

areina commented 10 years ago

Well, probably we need two layers, one to manage docsets and other to enable/disable them.

The management layer could contain these methods:

The enable/disable could have two modes.

what do you think?

kidd commented 10 years ago

I think it's ready to merge. Can anyone test it?

agpchil commented 10 years ago

Can't review your changes in detail right now but I did a quick test and didn't work well for me. It seems like helm-dash-filter-connections does something different now. It appens local docsets with common docsets so it doesn't allow to restrict them?

I expected helm-dash-filter-connections to filter connections using local docsets if set or use common docsets otherwise.

kidd commented 10 years ago

oopsie, sorry if it's not working. will give it another look... (It seemed to work fine in my machine (tm)).

The idea for now is that there is a global list for docsets, and local ones. the filter is by the union of both lists.

Will expand later also.

Thx for your time though

agpchil commented 10 years ago

Thanks to you for coding this! :)

I'll try to give an example of my use cases but as I said I didn't follow your changes in detail so maybe I'm wrong.

Having two global docsets A and B:

  1. You want to add a C docset in a local buffer. (helm-dash will show: A + B + C)
  2. You want to show only A docset in a local buffer. (helm-dash will show: A)

With latest changes case 1) seems to work (because it does local-docsets + global-docsets). But case 2) don't work because B is also present in global-docsets. So it will show A and B.

If helm-dash-filter-connections filters by local docsets (if it's set) then you can set local-docsets as global-docsets + C (case 1) or set local-docsets to A to only show A docset (case 2).

kidd commented 10 years ago

yup, 1) is what should happen. I also thought having just one variable which is global and localized when modified, but there were some edge cases that weren't clear to me. I'd say we can go with it for now, and maybe change it after some time when we'll all have more experience with those use cases.

areina commented 10 years ago

hey, it works fine for me. I'm going to merge it.

agpchil commented 10 years ago

oops, I'm late. Anyway, I'll answer my own question for anyone else interested. My two use case examples can be achieved using only helm-dash-docsets like:

;; set docsets for all buffers
(setq helm-dash-docsets '("Ruby" "Redis"))

;; customize for ruby-mode
(add-hook 'ruby-mode-hook '(lambda ()
   ;; case 2
   ;; (setq-local helm-dash-docsets (append helm-dash-docsets '("C")))

   ;; case 1
   (setq-local helm-dash-docsets '("Ruby")))

Probably this is not how helm-dash-docsets should be used but at this moment this works better for me.