search

GiselleSerate / myaliases

Useful shell aliases and functions.
6 stars 1 forks source link

added cds command to .bash_nav #11

Closed aryarm closed 6 years ago

aryarm commented 6 years ago

Not sure if you wanted this command I wrote, but I thought I'd create the pull request anyway!

The cds command allows you to create shortcuts in your bash. It's my version of cd with the added functionality of shortcuts. These shortcuts allow you to jump to a previously bookmarked directory by typing cds <shortcut_name> no matter where you are. You can create a shortcut by typing cds + <shortcut_name> in the directory that you want to bookmark. This will also override any existing shortcuts with the same name. You can delete a bookmark by typing cds - <shortcut_name> anywhere. You can list all available shortcuts by running cds .. It also has almost all the functionality of the regular cd command. I've actually aliased cd to cds on my local computer and have so far encountered little errors (except for issues #12 and #13). (As of the writing of this pull request, I recommend that y'all don't replace cd with cds until more of the kinks have been worked out.)

So why do I use these shortcuts instead of bash aliases? The cds command lets me create and delete shortcuts really quickly. No need to navigate to your .bash_nav file, open vim, copy and paste the path, and then source bash. I can create shortcuts that I'll only use for the next two minutes. I guess there's also the benefit of this being user-friendly (especially for beginners); you don't need to know how to create bash aliases to create cd shortcuts.

GiselleSerate commented 6 years ago

EDIT: Reviewing both PRs, I didn't read the other one lol.

GiselleSerate commented 6 years ago

I'm not yet convinced that everything is foolproof; I think I need to perform more rigorous testing on this before we can totally replace cd, of course, but I want to at least sleep on the changes I've made and come back to this at some point in the future.

aryarm commented 6 years ago

It might be a very long time before we feel confident that the cds command can replace cd. And frankly, it might even be a personal choice. Also, regardless of whether we can replace cd with cds, we at least feel confident that cds works (ie it does shortcutting and everything), right? Then isn't that enough to justify merging it in? For now, I think we should merge it in and replace cd with cds on our machines. If bad things happen, we can fix them. If nothing happens after a month or two, then we can consider adding alias cd='cds' to the remote version. I'll also try to come up with obscure test cases where things might break.

GiselleSerate commented 6 years ago

Oh, I wasn't quite to the point where I was confident that cds worked 100%; I wanted to do a little more testing first. I have the cds_nav branch running on my desktop right now, and I was going to keep using it for a bit before merging in. Particularly the quotes bit, I don't think it should break stuff but I really just added that, so I'd like to subject it to a bit of testing first.

aryarm commented 6 years ago

Oh ok. Sounds good!

GiselleSerate commented 6 years ago

Has this remained unproblematic enough for me to pull it?

aryarm commented 6 years ago

Yes and no. It's worked just fine on knuth and my bash terminal. But I learned just yesterday that it completely kills cd on my phone's terminal emulator if you alias cd to it. Also I still have a pending enhancement in Issue #13. I would recommend pulling it, but not yet replacing cd with it. It technically works perfectly for what it's supposed to do. It's only when you try to replace cd with cds that you sometimes get some unexpected behavior. Has it been working for you?

GiselleSerate commented 6 years ago

Yeah, I mean I haven't been using it as much as you have, but I haven't run into any problems. Do you think your phone is enough of an edge case that you want to merge or would you rather wait until you have everything fixed? Also, there's merge conflicts now, so we should fix them, but details.

aryarm commented 6 years ago

I mean, regardless of whether my phone is enough of an edge case, the current pull request just enables cds. It doesn't replace cd with cds, and cd on my phone only stopped working when I replaced cd with cds. But cds has been working on its own without any hiccups in every situation I've tested so far. I don't think this pull request should deal with making cds good enough to replace cd. That should be a different iteration, especially because it may take much longer before that feature is good to go.