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

Remove shell rc eval step #12

Closed
acrisci opened this issue Apr 2, 2016 · 5 comments
Closed

Remove shell rc eval step #12

acrisci opened this issue Apr 2, 2016 · 5 comments

Comments

@acrisci
Copy link
Contributor

acrisci commented Apr 2, 2016

I believe this project would be much more user friendly and easier to maintain if we could somehow get rid of the step that requires users to add command -v dtags > /dev/null 2>&1 && eval "dtags shell zsh" to their shell rc. The alternative would be to add an executable for d into the PATH instead of relying upon an alias.

This seems to be a deliberate design decision on your part so I assume you did it for a reason. Could you please explain your thought process behind this decision?

If i went through the code more carefully and found a way to accomplish this without the eval step, would you consider a PR for this feature?

@joowani
Copy link
Owner

joowani commented Apr 2, 2016

Hi acrisci,

As far as I know, there is no way to change the working directory without using the shell built-in command cd. And the only way to call cd in your current process is to do it "through the shell" (e.g. alias, function). In other words, there is no way to achieve this with a python (or any other) executable. We would have to use a shell script instead. Even if we did have this d shell script, in order to add it to PATH would still result in the user having to add a line in his/her runtime configuration (not 100% sure). If you can find a way to avoid this I would be more than happy to consider it.

Jay

@acrisci
Copy link
Contributor Author

acrisci commented Apr 2, 2016

I didn't think of that. It turns out you are correct.

There is another way that might be a bit cleaner though. You can also change directories by sourceing a shell script that changes directories. So for instance, if chdir.sh contains

cd $1

I can do source chdir.sh ./some-dir to go to the directory. So an alternative to the shell-specific eval, you could distribute /usr/bin/dtags-chdir.sh:

cd $(/usr/bin/dtags-chdir.py $1)

Where dtags-chdir.py is a python script that will echo where to go based on the input.

The benefit of that would be that you could put all your logic in the python script to avoid repeating your logic in many different places. It also has the benefit of simplifying the instructions to telling people to put this in their shellrc file:

alias d="source /usr/bin/dtags-chdir.sh"

for every shell (including the ones you don't support now). Backwards compatability could be achieved by evaling that alias when dtags shell [shell] is evoked.

This is just a thought on how to make things easier. If you like things the way they are, that's fine :).

@joowani
Copy link
Owner

joowani commented Apr 2, 2016

First of all thanks a lot for all your input. I really appreciate it.

I have considered what you are suggesting (a shell function/script piggybacking on a python executable) in the past, and you are right it definitely has its advantages. The only thing I didn't like about the approach was the fact the python interpreter would be invoked every single time. I've actually tested both methods and as small as it may be, I noticed a small difference in speed (especially on slow VMs). I decided to sacrifice a little bit of convenience during set up in exchange for better overall performance. In fact, in the future I may leave Python altogether and convert the whole tool into a set of shell functions for this very reason.

@acrisci
Copy link
Contributor Author

acrisci commented Apr 2, 2016

That is actually a good point. Every milisecond counts when running a command like cd and I'm sure I would notice the difference.

There can be a solution to that, however, that you should consider before you ditch python. It is possible to compile the python files so that they load faster. You should see a big difference in startup times. Whether or not that's good enough I'll leave up to you.

https://docs.python.org/release/1.5.1p1/tut/node43.html

@joowani
Copy link
Owner

joowani commented Apr 2, 2016

The conversion will probably take a lot research and time to carry out. So it won't happen any time soon, if at all. I've never considered .pyo files. I will try it out when I have time. Thanks!

@joowani joowani closed this as completed Apr 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants