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

Support flow's annotations #224

Open
dgellow opened this issue Mar 11, 2015 · 23 comments
Open

Support flow's annotations #224

dgellow opened this issue Mar 11, 2015 · 23 comments

Comments

@dgellow
Copy link

dgellow commented Mar 11, 2015

I want to use optional type annotation from Facebook's flow.
Any idea how it can be done with js2-mode?

@dgutov
Copy link
Collaborator

dgutov commented Mar 11, 2015

That's not possible now, and it doesn't look easy to implement.

@dgellow
Copy link
Author

dgellow commented Mar 11, 2015

I understand it's not an easy problem. I think typescript users are in a similar situation. js2-mode is the best js mode available, it's frustrating to leave it when you want to add annotations.
Would it be simpler to ignore them (no syntax highlighting, etc.) ? It would be better than the current situation:
screenshot_annotated_javascript

@dgellow
Copy link
Author

dgellow commented Mar 11, 2015

I have just noticed that flow 0.4.0 supports annotations in comments:

As of Flow 0.4.0, you can put your Flow-specific syntax in special comments. If you use these special comments then you do not need to transform away Flow-specific syntax before running your code.
[...]
This feature introduces 3 special comments: /:, /::, and /*flow-include. Flow will read the code inside these special comments and treat the code as if the special comment tokens didn't exist

@dgutov
Copy link
Collaborator

dgutov commented Mar 11, 2015

I think typescript users are in a similar situation. js2-mode is the best js mode available, it's frustrating to leave it when you want to add annotations.

On the other hand, once you use something like TypeScript, you already have better type safety, and it should be reasonably simple to hook up with something like Flycheck, and some basic assumptions that some modes that use JS2 AST (like skewer-mode) make won't hold anyway.

Likewise, TypeScript et al should have a better tooling support, so it'd be better to integrate those instead of using js2-refactor. Considering that, js-mode should be adequate as the major mode.

As of Flow 0.4.0, you can put your Flow-specific syntax in special comments.

Indeed, that should be the other natural approach, as long as there's no existing project you have to work on, that uses the simple syntax.

@felipeochoa
Copy link
Contributor

I was looking into this (foolishly) thinking there might be a way using advice to layer this on top of js2, like I did with jsx, but this syntax obviously hits many more places, rendering that strategy useless.

Another thought is to create a 3rd party package that parses and supports better fontification of flow comments, which are easy to recognize. We could start by just supporting variables/params using the /*: type */ syntax and maybe generics using /*::<T, U, V>*/ syntax. Maybe eventually everything else inside /*flow-include */

I don't have time right now to push this forward, but wanted to throw the idea out there as it might be a nearer-term solution than updating js2 itself to handle the syntax.

@dgutov
Copy link
Collaborator

dgutov commented Oct 26, 2016

If you were looking for an entry point, advising js2-highlight-jsdoc might help.

@aaronjensen
Copy link

While just using js2-mode (well, rjsx-mode) for indentation and syntax highlighting, I don't run into too many issues with flow annotations. Refactors don't work. Also, indentation of the exact hamburgers is broken:

export type SessionState2 = {|
                             loaded: boolean,
                             currentUser: ?User,
                             |}

I'm 👎 on a mode for coloring the comment syntax, I'd rather effort was put into supporting the regular syntax. Would that necessitate a fork of js2-mode? Or would js2-mode consider supporting it optionally (if that's even possible w/o much disaster)

@dgutov
Copy link
Collaborator

dgutov commented Dec 20, 2016

Someone could try an see if that's feasible to implement with advice, similar to rjsx-mode. I'm not sure we'd want to merge it to this repo later, though. Maybe if the logic is cleanly separated...

@jedahu
Copy link

jedahu commented Feb 2, 2017

Quick and dirty advice for hamburger indentation and js and js2 modes.

It's a copy-paste and modify, so real dirty.

    (defun jdh--js--proper-indentation (_ parse-status)
      "Return the proper indentation for the current line."
      (save-excursion
        (back-to-indentation)
        (cond ((nth 4 parse-status)    ; inside comment
               (js--get-c-offset 'c (nth 8 parse-status)))
              ((nth 3 parse-status) 0) ; inside string
              ((eq (char-after) ?#) 0)
              ((save-excursion (js--beginning-of-macro)) 4)
              ;; Indent array comprehension continuation lines specially.
              ((let ((bracket (nth 1 parse-status))
                     beg)
                 (and bracket
                      (not (js--same-line bracket))
                      (setq beg (js--indent-in-array-comp bracket))
                      ;; At or after the first loop?
                      (>= (point) beg)
                      (js--array-comp-indentation bracket beg))))
              ((js--ctrl-statement-indentation))
              ((js--multi-line-declaration-indentation))
              ((nth 1 parse-status)
               ;; A single closing paren/bracket should be indented at the
               ;; same level as the opening statement. Same goes for
               ;; "case" and "default".
               (let ((same-indent-p (looking-at "[]})]\\||}")) ;; JDH modified regex
                     (switch-keyword-p (looking-at "default\\_>\\|case\\_>[^:]"))
                     (continued-expr-p (js--continued-expression-p)))
                 (goto-char (nth 1 parse-status)) ; go to the opening char
                 (if (looking-at "\\([({[]\\|{|\\)\\s-*\\(/[/*]\\|$\\)") ;; JDH modified regex
                     (progn ; nothing following the opening paren/bracket
                       (skip-syntax-backward " ")
                       (when (eq (char-before) ?\)) (backward-list))
                       (back-to-indentation)
                       (js--maybe-goto-declaration-keyword-end parse-status)
                       (let* ((in-switch-p (unless same-indent-p
                                             (looking-at "\\_<switch\\_>")))
                              (same-indent-p (or same-indent-p
                                                 (and switch-keyword-p
                                                      in-switch-p)))
                              (indent
                               (cond (same-indent-p
                                      (current-column))
                                     (continued-expr-p
                                      (+ (current-column) (* 2 js-indent-level)
                                         js-expr-indent-offset))
                                     (t
                                      (+ (current-column) js-indent-level
                                         (pcase (char-after (nth 1 parse-status))
                                           (?\( js-paren-indent-offset)
                                           (?\[ js-square-indent-offset)
                                           (?\{ js-curly-indent-offset)))))))
                         (if in-switch-p
                             (+ indent js-switch-indent-offset)
                           indent)))
                   ;; If there is something following the opening
                   ;; paren/bracket, everything else should be indented at
                   ;; the same level.
                   (unless same-indent-p
                     (forward-char)
                     (skip-chars-forward " \t"))
                   (current-column))))

              ((js--continued-expression-p)
               (+ js-indent-level js-expr-indent-offset))
              (t 0))))

    (defun jdh--js--looking-at-operator-p (_)
      "Return non-nil if point is on a JavaScript operator, other than a comma."
      (save-match-data
        (and (looking-at js--indent-operator-re)
             (not (and (looking-at "|") (eq ?\{ (char-before)))) ;; JDH added
             (or (not (eq (char-after) ?:))
                 (save-excursion
                   (and (js--re-search-backward "[?:{]\\|\\_<case\\_>" nil t)
                        (eq (char-after) ??))))
             (not (and
                   (eq (char-after) ?*)
                   ;; Generator method (possibly using computed property).
                   (looking-at (concat "\\* *\\(?:\\[\\|" js--name-re " *(\\)"))
                   (save-excursion
                     (js--backward-syntactic-ws)
                     ;; We might misindent some expressions that would
                     ;; return NaN anyway.  Shouldn't be a problem.
                     (memq (char-before) '(?, ?} ?{))))))))

    (advice-add 'js--proper-indentation :around #'jdh--js--proper-indentation)
    (advice-add 'js--looking-at-operator-p :around #'jdh--js--looking-at-operator-p)

@dgreensp
Copy link
Contributor

dgreensp commented Feb 9, 2017

I'm considering trying to implement Flow support.

What is rjsx-mode, and what is "advice"?

@dgutov
Copy link
Collaborator

dgutov commented Feb 9, 2017

@dgreensp Check out https://github.com/felipeochoa/rjsx-mode.

See the advice-add and advice-remove calls there.

@dgreensp
Copy link
Contributor

dgreensp commented Feb 9, 2017

Oh, "advice" is an Emacs Lisp feature that lets you wrap an existing function in place, like a monkey-patch.

@dgutov
Copy link
Collaborator

dgutov commented Feb 9, 2017

Yep. Except it's easier to undo cleanly.

@fxfactorial
Copy link

@dgutov what is the current solution for getting correct syntax highlighting in emacs for flow? Anything?

@dgutov
Copy link
Collaborator

dgutov commented Jul 19, 2017

@fxfactorial
Copy link

bah, former looks like it's more for auto complete/type checking, and latter says:

'It is a major mode that is a replacement for js2-mode and js-mode.' which is off putting. =/

@dgutov
Copy link
Collaborator

dgutov commented Jul 19, 2017

What did you expect? Major modes perform syntax highlighting.

@fxfactorial
Copy link

Expected a minor mode that maybe patched js2

@dgutov
Copy link
Collaborator

dgutov commented Jul 19, 2017

If somebody did that, they'd probably leave a comment here.

@pawelk
Copy link

pawelk commented Oct 19, 2017

There's flow-minor-mode but it doesn't provide syntax highlighting in js2-mode, see this issue.

@felipeochoa
Copy link
Contributor

felipeochoa commented Oct 19, 2017

Here's the proof-of-concept that @antifuchs mentions in that thread. I haven't tried it, but there's a minor mode there that advises several js2 parsing functions to handle some flow syntax.

@antifuchs
Copy link

Ah, yep, that's right - sorry I forgot to post this. My branch can handle some flow-related syntax, but definitely not all the relevant syntax, sadly. Most notable is the lack of function argument annotation support, which I fear will require a major restructuring of the way js2-mode represents functions. I'm currently not able to spend any time on this anymore, but if somebody wants to take over development of this, I'd love to hand off editing permissions on the branch to you!

@Fuco1
Copy link
Contributor

Fuco1 commented Oct 27, 2017

I've started hacking a bit on Andreas's fork and opened a PR to his repo: antifuchs/rjsx-mode#1 If anyone's interested doing a bit of a code review I would appreciate that. I'm only beginning with flow so some solutions might not work in each construct, but we can figure that on the way.

I have a list of things that break as I keep working with flow, if you have some other pain-points add your own list in a comment and I will merge them to the OP so we can keep track of what needs to be done.

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

No branches or pull requests

10 participants