search

JonyEpsilon / gorilla-repl

A rich REPL for Clojure in the notebook style.
http://gorilla-repl.org
MIT License
887 stars 104 forks source link

Save as ... #155

Closed jococo closed 10 years ago

jococo commented 10 years ago

'Save As' feature. Have commented out combo('g', 'a') for autocomplete to use with Save As. Feel free to change if required.

Ignore project.clj file. Am using it locally while developing and wasn't sure how to exclude it from pull request.

ghost commented 10 years ago

I don't think one can simply ignore a part of the changeset like the profile.clj in this case. I would also highly recommend to not put multiple issues into one pull request.

Not my project but I'd recommend cleaning up the pull request and then resending it :/

jococo commented 10 years ago

Yeah, no problem. I'm still a bit green when it comes to github pull requests. I actually committed the code for the second issue after the first pull request so was quite surprised when it got added.

Will fix it tonight.

JonyEpsilon commented 10 years ago

Thanks @jococo for the PR :-) And thanks @ticking for the comments. As a rule, I'd say I'm always happy to receive PRs, even if they are rough around the edges. I'd much rather someone felt able to contribute, rather than be put off by having to get things spot-on first time. I remember my first attempts at PRing and rebasing, and it's certainly not intuitive!!

The save as stuff looks good. Do you want to extract just that part into this PR and rebase it against the current development branch? I can walk you through it if you don't know how yet. Or, I'm happy to do it myself, but if you'd like to try it, then feel free - just let me know.

The CSS stuff, I'm not so sure on. I'll comment more on the issue #130.

JonyEpsilon commented 10 years ago

Ohh, and I think we should find another key combo than g, a as it would be better not to change that for autocomplete. Unfortunately ctrl+space doesn't work on Firefox, so there needs to be an alternate command for those users.

jococo commented 10 years ago

Yeah I 'm quite happy to fix up the PR tonight. It's the only way I will learn to do it right.

As for the keys, I just thought it's reasonably common to use the 'a' for 'save As', other times I see 'SHIFT+S' but I couldn't get that to work. What do you think of defining the following three:

g + c => 'auto Complete' # it's nearer to the space key. g + d => 'clojure Docs' g + a => 'save As'

These are just suggestions and I'm not particularly precious about any decision, just trying to help achieve something that's easy to remember.

JonyEpsilon commented 10 years ago

Great, let me know if I can help with any of the git incantations :-)

Regarding the shortcuts, I think I agree with your proposed assignment in principle - it seems sensible and easy to remember - but I'd rather not break compatibility with the past, so I think we'll have to settle for something less sensible! Also note that g + d is already used for "move segment down". (And, you're probably aware of it, but you customise the keymap per user, or per project, in profiles.clj).

jococo commented 10 years ago

Thanks. The one issue I don't understand is how I keep the fork up to date without getting those merge entries which then fed back into the pull request.

I understand the issue with picking keys that everyone can agree on but I can't think of a reasonable one for 'save as' that is both convenient and memorable/logical and similar to key commands that other applications use. Am happy to maintain my own user keymap but I wonder whether you would mind if I added a page to the website with a table of all the standard key combinations?

JonyEpsilon commented 10 years ago

You can git rebase to target the PR to the latest develop without merging: http://git-scm.com/book/ch3-6.html . I'll walk through it on the other PR.

As for the keymap page, I'm not sure it's the right way to go. At the moment, you can bring up the menu in Gorilla (ctrl+g twice, or hit the icon) and it gives you a list of all the commands along with any keyboard shortcut mapped to them. My feeling is that a static page on the website would be less good than this, and would be a maintenance burden to keep in sync. Is there something about the menu that you don't find convenient for looking up shortcuts? If so, we might be better to address that.