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

Adding first version that allows parsing the ssh config files for hosts #24

Merged
merged 3 commits into from
Jul 12, 2013

Conversation

grundprinzip
Copy link
Contributor

This is the first proposal for #14 to include all hosts that are already configured in the ssh config file. Currently namespaces identified with "/" are not supported.

Let's discuss.

@grundprinzip
Copy link
Contributor Author

I tried to address all your comments, with regards to filtering entries like * this was because my ssh config has an entry for special configuration of EC2 hosts and uses a * here to explicitly include all different possible subdomains.

@fitztrev
Copy link
Owner

I like this approach. I had considered removing the json config entirely but this might be better.

In the menuWillOpen trigger, there's a check to see if the config file has been updated. If it has, it'll refresh the menu with the new content. I like that because the user doesn't have to do anything (hit a "reload" button, or restart the app) to get the changes to show up. Could that be added for this as well?

One of the things I noticed was the same problem I had when figuring how to construct the JSON. Dictionary containers don't honor the order of their contents. So the hosts in the config file aren't guaranteed to end up in that same order. I ended up making the hosts an array with each value being its own object. Not sure if this is the best way to do it, but I think we may want respect the order that the user sets in their config.

@grundprinzip
Copy link
Contributor Author

For the check of the last changed date I will add this in the same way as it is done for the json file.

For the order of the entries I basically extract all keys from the dictionary and sort them so the order in the menu is always alphabetically which seemed to me like the right thing to do...

@fitztrev
Copy link
Owner

👍 Agreed. Alphabetical is good.

Adding hosts from SSH Config so they don't need to be configured twice
in the system. You can use "/" to separate namespaces and create
virtual folders in the menu.
@grundprinzip
Copy link
Contributor Author

Ok, I added the mtime handling and squashed the commit so its nice and mergable :)

@fitztrev
Copy link
Owner

Awesome, thanks. :)

When testing, needUpdateFor was returning that ~/.ssh/config did not exist. When I changed it to the following in menuWillOpen it worked:

[NSHomeDirectory() stringByAppendingPathComponent:@".ssh/config"]

Also, I don't use the global ssh_config on my machine but mine's located at /etc/ssh_config. Not sure how it is for other people or how they use it.

@grundprinzip
Copy link
Contributor Author

You're right, there was a bug in the needUpdateFor method as it was not expanding the tilde in the path, I fixed that. Same goes for the system location of the ssh config. The man page states that the ssh config is located in /etc/ssh/ssh_config but it actually is right where you pointed. I didn't check because I assumed it would be empty.

fitztrev added a commit that referenced this pull request Jul 12, 2013
@fitztrev fitztrev merged commit 104c118 into fitztrev:master Jul 12, 2013
@fitztrev
Copy link
Owner

👍 Thanks!!

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

Successfully merging this pull request may close these issues.

None yet

2 participants