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

Emacs plugin rewrite #2261

Closed
wants to merge 2 commits into from
Closed

Emacs plugin rewrite #2261

wants to merge 2 commits into from

Conversation

afonso360
Copy link

@afonso360 afonso360 commented Aug 16, 2016

Instead of defining a few functions and using global-set-key possibly overriding users keybindings, I've created a proper minor mode with local keybindings and prefixes.

I also added company-mode support for autocompletion, it works but some work is still required.

As we init flow servers we keep track of their existance, this is still not used but eventually I would like to have a variable to enforce a flow server before executing each command and the possibility to kill all flow servers tracked by Emacs

Fixes #724 except for flow-types-fill-types

@ghost
Copy link

ghost commented Aug 16, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@ghost
Copy link

ghost commented Aug 16, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed label Aug 16, 2016
@avikchaudhuri
Copy link
Contributor

Thanks @afonso360! I'll take a look. Meanwhile I modernized the existing plugin a bit, but didn't go as far as creating a minor-mode. This looks pretty cool. Would be nice to merge these efforts and launch a separate project to maintain the plugin, WDYT?

@ghost ghost added the CLA Signed label Aug 16, 2016
@afonso360
Copy link
Author

Sounds like a good idea, also closes #999

@FossiFoo
Copy link

FossiFoo commented Aug 24, 2016

yay. the more the merrier. this looks almost identical to the minor-mode we whipped up.
i'd be happy to contribute some various functionality (covered line highlighting, diff merging) too.

but could somebody from facebook, please, please, pretty please open up a repo for this? see #999

@FossiFoo
Copy link

@avikchaudhuri
Copy link
Contributor

We're in the process of creating a repo and inviting contributions. Thanks
for your patience!

On Wednesday, August 24, 2016, FossiFoo [email protected] wrote:

my current version is here btw: https://github.com/FossiFoo/.
emacs.d/blob/master/site-lisp/flowtype-mode/flowtype-mode.el


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#2261 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFFNMXZjmigJ9e9FkuYJaTEP-qSlg8a0ks5qjGkugaJpZM4JlnCQ
.

@avikchaudhuri
Copy link
Contributor

@afonso360 Could you please share install instructions? I'm using GNU Emacs 24.5.1. For some reason after M-x global-flow-types-mode whenever I type the character f I get "Symbol's function definition is void: string-of-region." When I define that function I get the same error for flow-binary. Are you relying on the existing definitions in flow-types.el somehow?

@ghost ghost added the CLA Signed label Aug 25, 2016
@avikchaudhuri
Copy link
Contributor

@FossiFoo Could you also please share install instructions so that I can play around? There are a bunch of dependencies. Which version of emacs is your mode built for?

@ghost ghost added the CLA Signed label Aug 25, 2016
Add missing string-of-region function and prevent minor mode
keybindings from leaking into the regular user map
@afonso360
Copy link
Author

@avikchaudhuri Yes I was relying on those functions because I didn't want to remove existing functionality, I've pushed a commit that should fix the missing function definitions and prevent the keybindings from leaking. (they were being bound without the prefix).

As for install instructions, placing this on the config works, but it enables flow globally, which might not be the best solution.

(add-to-list 'load-path "~/git/flow/")
(require 'flow-types)
(global-flow-types-mode)

If you don't want flow enabled globally, replacing global-flow-types-mode with

(add-hook 'web-mode-hook 'flow-types-mode)

Will only enable flow-types on the major mode Web-mode.

@ghost ghost added the CLA Signed label Aug 25, 2016
@FossiFoo
Copy link

Any progress on the repo?

I'm not aware of any special things that need to be done or any special emacs version required. I'm currently using 24.5.2 but it also works (maybe with patches?) with spacemacs and aquamacs. All "needed" packages can be fetched from MELPA, but it should run fine without flycheck or company for example.

Of course this is so far not polished at all and for my private use, so i didn't clean up for a while. :)

@gabelevi
Copy link
Contributor

Sorry for the delay - I'm pinging @avikchaudhuri and seeing what's blocking us from publishing the emacs repository. Thanks for your patience!

@FossiFoo
Copy link

cool. a repo with any start would still be a great thing... :)

@gabelevi
Copy link
Contributor

Ok! It's up! Let's all move to https://github.com/flowtype/flow-for-emacs/ !

As for this PR, I'm going to close it out. My apologies again for the delay in getting that repo published!

@gabelevi gabelevi closed this Oct 28, 2016
@FossiFoo
Copy link

FossiFoo commented Nov 1, 2016

can you then please also remove the emacs file from this repo? it's just confusing to have that around if the official source is now elsewhere.

@gabelevi
Copy link
Contributor

gabelevi commented Nov 1, 2016

@FossiFoo - sure! Just created #2731. Thanks for the reminder!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use --json in emacs plugin
4 participants