search

cliffrowley / atom-open-in-github-app

Atom package to open the current project in GitHub.app
MIT License
5 stars 1 forks source link

Fixed Issues #11

Closed tcyrus closed 9 years ago

tcyrus commented 9 years ago

Fixed Issues #10, #8, #7, and #6

jrsconfitto commented 9 years ago

Thanks for this! This looks much simpler than how i was trying to fix things. i’ll try to give it a review tomorrow morning.

cliffrowley commented 9 years ago

FYI I have already done some work on this in the multiple-project-support branch, but I really don't like the way the Windows GitHub app behaves and I couldn't seem to get it to play along.

On the Mac there are two ways to launch the app with a local path, either with open -a <path/to/app> <project_dir>or by launching the URL github-mac://<project_dir>, where project_dir does not have to be an URL, but can simply by the (absolute) path to the local repository.

On Windows it seems that no matter how I try to launch the app, I can not get it to do so given a local repository. If I do so, it just opens on whatever repository was last selected. If I give it an URL, it wants to clone it every time whether or the app is already aware of the repository or not.

Very frustrating. Any ideas? How do you expect it to behave on Windows? They are two very (very) different apps.

cliffrowley commented 9 years ago

Also this PR doesn't satisfy #7 (have a look at my multiple-project-support branch and see how I handle it there).

Also we really appreciate the PR - thanks for that - but be aware that if an issue is assigned then it's probably being worked on (though there's no harm in asking!). Don't remove what you've done, though, I will certainly review your code and incorporate where appropriate :-)

jrsconfitto commented 9 years ago

i think it's worth incorporating some of the code from this. If i'm reading it correctly, this will open the repository which the current open file is part of. i find that super useful and pretty much always what i intend.

i think we'll still need to have the user select which repository they mean to open, but i think the default should be what i mention in the previous paragraph.

jrsconfitto commented 9 years ago

On Windows it seems that no matter how I try to launch the app, I can not get it to do so given a local repository. If I do so, it just opens on whatever repository was last selected. If I give it an URL, it wants to clone it every time whether or the app is already aware of the repository or not.

Very frustrating. Any ideas? How do you expect it to behave on Windows? They are two very (very) different apps.

As i mentioned in my comment on your branch commit (sorry i didn't respond here), all that stuff finding the origin url and the branch name was so this stuff could get close to working... i find it only likes working for GitHub-based repos that are already registered with the application :frowning:

cliffrowley commented 9 years ago

@jugglingnutcase I see what you're saying, but there's a couple of problems with the 'open for the current file' use case:

Firstly that on Windows the GitHub app is really quite stupid and will open a new instance every time (rather than activate an existing instance even if its using the same repository!) and it's really quite slow to launch, so typically the path of least resistance is going to be to open the app for a project directory (and then leave it open).

Secondly that it really muddies the users intention. Say I've got a project open with 2 or more folders, and I have some files open. I've modified files from one of the folders, but I haven't currently got any of those files open in the editor, though I do have unmodified files open from another. If I ctrl-alt-g now then the app will open for the folder in which I haven't actually modified any files and in order to open the right folder I would need to either remember to open a file from the correct folder or close all the files and pick a project dir from the popup list.

Therefore I think personally (therefore obviously totally open for debate!) that we should maintain a single level of granularity - the project folder, and then it's always clear what the user is getting. If they have 1 project folder, the app will open for that folder, and if they have 2 or more, they get a choice as to which to open. Keeps it simple and predictable.

cliffrowley commented 9 years ago

@jugglingnutcase regarding the code to grab the origin URL, this functionality now exists in the Atom API (which I am using here https://github.com/cliffrowley/atom-open-in-github-app/blob/multiple-project-support/lib/open-github-app.coffee#L13). Is there anything needed beyond that?

cliffrowley commented 9 years ago

@jugglingnutcase we really need to put this to bed but I'm hesitant to do so without a resolution to our discussion here first (otherwise this isn't much of a collaboration if I go ahead and do whatever I think!) :-) I'm thinking that since we have two very distinct usage patterns (e.g. the app behaves differently on Windows and Mac) we should document this. I will do that for Mac now (in the multiple-project-support branch), if you wouldn't mind taking a look when I'm done and doing the same for Windows?

cliffrowley commented 9 years ago

Could you please check the multiple-project-support branch as I'm now having issues with my Windows GitHub client (I officially hate now it as much as I hate Windows :P) and I don't even seem to be able launch the app via the "Clone to desktop" button on the website.

cliffrowley commented 9 years ago

Actually, never mind - it seems the app is broken on both my Windows installations (one virtual, one metal). I'm just going to go ahead and merge the branch in and deal with any issues afterwards, it can't hurt since the Windows code is already broken.

cliffrowley commented 9 years ago

Incorporated by https://github.com/cliffrowley/atom-open-in-github-app/pull/12

jrsconfitto commented 9 years ago

Sorry I didn't get back to this sooner. I'll have some time today to take a look into the Windows stuff and see if I can help it move along. On Wed, Jun 10, 2015 at 11:53 PM Cliff Rowley notifications@github.com wrote:

Closed #11 https://github.com/cliffrowley/atom-open-in-github-app/pull/11.

— Reply to this email directly or view it on GitHub https://github.com/cliffrowley/atom-open-in-github-app/pull/11#event-328110136 .