Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG+1] Added zsh completion for Scrapy command-line tool and updated bash completion #934

Merged
merged 6 commits into from Jul 30, 2015
Merged

[MRG+1] Added zsh completion for Scrapy command-line tool and updated bash completion #934

merged 6 commits into from Jul 30, 2015

Conversation

@Dineshs91
Copy link
Contributor

@Dineshs91 Dineshs91 commented Nov 2, 2014

Auto completion for zsh.

@dangra
Copy link
Member

@dangra dangra commented Nov 6, 2014

I don't use zsh, but I don't any objections to merge this. Anyone else want to review?

@@ -1 +1,2 @@
extras/scrapy_bash_completion etc/bash_completion.d/
extras/_scrapy_zsh_completion /usr/local/share/zsh/site-functions/

This comment has been minimized.

@nyov

nyov Dec 17, 2014
Contributor

/usr/local is not used in Debian for a zsh install.
Looks like this would belong in /usr/share/zsh/vendor-completions/_scrapy

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Dec 30, 2014

@nyov
extras/_scrapy_zsh_completion /usr/share/zsh/vendor-completions/
will this change work ?

@nyov
Copy link
Contributor

@nyov nyov commented Dec 30, 2014

I don't know, I'm not using zsh myself. Could you test it?
I think best would be, if it works,
extras/scrapy_zsh_completion usr/share/zsh/vendor-completions/_scrapy_zsh_completion
(no underscore in the filename before it gets installed).

@nyov
Copy link
Contributor

@nyov nyov commented Apr 2, 2015

Will this see an update, or should it get closed?

@rmax
Copy link
Contributor

@rmax rmax commented Apr 2, 2015

@Dineshs91 @nyov It works for me when copying the file to either /usr/share/zsh/vendor-functions/_scrapy_zsh_completion or /usr/share/zsh/vendor-completions/_scrapy. Notice that in the latter case the filename must be _scrapy.

@nyov
Copy link
Contributor

@nyov nyov commented Apr 2, 2015

Thanks for testing. it looks like I'm only having the vendor-completions folder on my system,
nothing else seems to have installed anything to vendor-functions or site-functions.
I would go with the latter version (also the filename of _scrapy instead of _scrapy_zsh_completion).
All together, extras/scrapy_zsh_completion /usr/share/zsh/vendor-completions/_scrapy

@curita
Copy link
Member

@curita curita commented Apr 3, 2015

Can I suggest we add this as a plugin for robbyrussell/oh-my-zsh too? It's a pretty standardized collector of plugins for zsh. Creating a folder under /plugins named scrapy and adding the _scrapy_zsh_completion script there should be enough.

Not sure if this helps since it's not debian based, but in arch completions scripts are installed under /usr/share/zsh/site-functions.

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Apr 3, 2015

@darkrho Thanks for your inputs !

@nyov I have updated the pull request.

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Apr 3, 2015

@curita Sure that's a great suggestion. I will create a PR later today.

It would be best, if the filename is _scrapy instead of _scrapy_zsh_completion. All plugins there, follow this convention. Completion file will be _pluginname.

arch completions scripts are installed under /usr/share/zsh/site-functions. This really helps. I am using a mac and this path (usr/share/zsh/vendor-completions/) doesn't exist, but /usr/share/zsh/site-functions is available.

;;
*)
if [[ CURRENT -eq 2 ]]; then
_arguments '*: :(check crawl deploy edit fetch genspider list parse runspider server settings shell startproject version view)'

This comment has been minimized.

@kmike

kmike Apr 3, 2015
Member

we are removing 'scrapy deploy' command, I think it shouldn't be listed here

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Apr 3, 2015

Can anyone confirm if server command is available ?

Like $ scrapy server

@kmike
Copy link
Member

@kmike kmike commented Apr 3, 2015

@Dineshs91 a good catch; scrapy server was removed a long time ago (it was a command for starting https://github.com/scrapy/scrapyd).

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Apr 3, 2015

@kmike Thanks for the prompt response.

These commands should be removed from scrapy_bash_completion also.

@nyov
Copy link
Contributor

@nyov nyov commented Apr 3, 2015

@Dineshs91

This really helps. I am using a mac and this path (usr/share/zsh/vendor-completions/) doesn't exist, but /usr/share/zsh/site-functions is available.

Don't forget you're adding this to the debian/ package path here, though. So for this PR, it should follow debian rules. For oh-my-zsh it's a different matter.

And I think no-one would mind if you fix scrapy_bash_completion as part of this ticket.

@Dineshs91
Copy link
Contributor Author

@Dineshs91 Dineshs91 commented Apr 4, 2015

  1. In detail

    $ scrapy [TAB]
    scrapy commands
    check         -- Check spider contracts,
    crawl         -- Run a spider,
    edit          -- Edit spider,
    
  2. Just the commands

$ scrapy [TAB]

check crawl edit fetch ....

Which one is better ?

@Dineshs91 Dineshs91 changed the title Added zsh completion for the Scrapy command-line tool Added zsh completion for Scrapy command-line tool and updated bash completion Apr 5, 2015
@curita curita changed the title Added zsh completion for Scrapy command-line tool and updated bash completion [MRG+1] Added zsh completion for Scrapy command-line tool and updated bash completion Apr 10, 2015
kmike added a commit that referenced this pull request Jul 30, 2015
[MRG+1] Added zsh completion for Scrapy command-line tool and updated bash completion
@kmike kmike merged commit 936266a into scrapy:master Jul 30, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants