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

add documentation for #192 #244

Merged
merged 3 commits into from
Dec 21, 2015
Merged

add documentation for #192 #244

merged 3 commits into from
Dec 21, 2015

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Dec 10, 2015

this includes #192

  • verbs: adding shell verbs 'cd' and 'source'

Shell support in ``catkin`` command
-----------------------------------

A bash/zsh user can use wrapper wrapper function, please read :doc:`development/catkin_shell_verbs`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"wrapper wrapper" looks like a typo.

@wjwwood
Copy link
Member

wjwwood commented Dec 10, 2015

Thanks for working on this @k-okada.

@k-okada
Copy link
Contributor Author

k-okada commented Dec 11, 2015

thanks for review, updated docs


.. code-block:: shell

. /opt/ros/indigo/etc/bash_completion.d/catkin_shell_verbs.{bash,zsh}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the right directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it is not correct ,and I think it is not installed, how can I configure setup.py to install ./scripts/catkin_shell_verbs.bash

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the right thing to do here is, maybe @jbohren can advise?

You could do something cute like this:

$ source `catkin --locate-extra-shell-verbs`

And then install the shell scripts that need to be sourced in with the Python code and provide an option to expose their final location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, how about this 25b8c4f, I'm not sure
how to get installed location ...
http://stackoverflow.com/questions/50499/how-do-i-get-the-path-and-name-of-the-file-that-is-currently-executing

◉ Kei Okada

On Sat, Dec 12, 2015 at 5:12 AM, William Woodall notifications@github.com
wrote:

In docs/advanced/catkin_shell_verbs.rst
#244 (comment):

@@ -0,0 +1,36 @@
+Shell support in catkin command
+===================================
+
+When you source the following file, you can use bash/zsh shell functions which provide added utility.
+
+.. code-block:: shell
+

  • . /opt/ros/indigo/etc/bash_completion.d/catkin_shell_verbs.{bash,zsh}

I'm not sure what the right thing to do here is, maybe @jbohren
https://github.com/jbohren can advise?

You could do something cute like this:

$ source catkin --locate-extra-shell-verbs

And then install the shell scripts that need to be sourced in with the
Python code and provide an option to expose their final location.


Reply to this email directly or view it on GitHub
https://github.com/catkin/catkin_tools/pull/244/files#r47399572.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly resource_filename is what you are looking for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know a reliable way to determine where the correct share/catkin_tools is to be found?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I found so far is follows:

 os.path.abspath(os.path.join(os.path.dirname(inspect.getfile(catkin_tools)), '../../../../share/catkin_tools'))
os.path.abspath(os.path.join(imp.find_module('catkin_tools')[1], '../../../../share/catkin'))

if we do not have other solution, we have to accept this ugly expresson, or put catkin_shell_verbs.bash under dist-packages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In [2]: resource_filename(Requirement.parse("catkin_tools"), "catkin_shell_verbs.bash")
Out[2]: '/home/k-okada/catkin_ws/ws_catkin/install/lib/python2.7/dist-packages/catkin_shell_verbs.bash'

In [3]: 
Do you really want to exit ([y]/n)? y
k-okada@kokada-t440s:~/catkin_ws/ws_catkin$ find install  -name catkin_shell_verbs.bash
install/share/catkin_tools/catkin_shell_verbs.bash

x-9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say just put it in dist-packages along side the code.

for arg in sysargs:
if arg == '--locate-extra-shell-verbs':
import inspect
print(os.path.abspath(os.path.join(inspect.stack()[-1][1],'../../','share/catkin_tools/catkin_shell_verbs.bash')))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks vestigial.

@wjwwood
Copy link
Member

wjwwood commented Dec 14, 2015

Looks like the CI is not passing, but other than a few comments it is looking good to me.

@wjwwood wjwwood merged commit 25b8c4f into catkin:master Dec 21, 2015
@wjwwood
Copy link
Member

wjwwood commented Dec 21, 2015

I fixed this up and merged it, thanks @k-okada!

@k-okada k-okada deleted the doc_source branch April 20, 2016 03:35
k-okada added a commit to k-okada/catkin_tools that referenced this pull request Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants