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

Nim-mode doesn't highlight functions as font-lock-function-name-face #217

Open
3 of 6 tasks
ogdenwebb opened this issue Oct 15, 2018 · 10 comments
Open
3 of 6 tasks

Comments

@ogdenwebb
Copy link

ogdenwebb commented Oct 15, 2018

  • basic information
    • Gentoo Linux, kernel 4.18.5
    • Emacs GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30) of 2018-08-24
    • Nim and nimsuggest version 0.18.0
  • Are you using latest version of nim-mode? Yes, 0.4.2 from MELPA.
  • Are there message related to nim-mode in *Messages* buffer? No.
  • Did you read README.md?
  • Is your problem related to nimsuggest-mode? (company-mode, flycheck, or el-doc)
    • Did you try nim-mode without nimsugggest-mode? What happened?
      (Comment out nimsuggest-mode's config please)

An example of nim-mode highlighting.

emacs-nim-mode

Go-mode behaviour in Emacs, also I notice VS Code has similar highlighting in nim files.

emacs-go-mode

@krux02
Copy link
Contributor

krux02 commented Oct 15, 2018

In many languages it is pretty easy to determine what a function is with a regular expression. In Nim this is not the case. For example in a.b a could be a package and b a function. a could be an object and b a member or a could be an object and b a function.

In a(b), a could be a function, a template or a macro.

Since highlighting is based on regular expressions, do you want every identifier before an open brace highlighted as a function name?

@ogdenwebb
Copy link
Author

ogdenwebb commented Oct 15, 2018

do you want every identifier before an open brace highlighted as a function name?

Probably we can do this, because there's nimsuggest-show-doc to describe a kind of object.

Since highlighting is based on regular expressions, do you want every identifier before an open brace highlighted as a function name?

Well, I don't know enough about Emacs syntax highlighting, but emacs-lisp has the same syntax for macros and functions, i.e. (identifier [args]), but Emacs treat a macro as keyword and a function as functions(in terms of the font-lock faces).

js2-mode treats identifier.x without brackets as js2-object-property and identifier.x() as js2-function-call. Ugh, I know that won't fit well into nim uniform function call syntax.

@krux02
Copy link
Contributor

krux02 commented Oct 15, 2018

syntax highlighting in emacs works search based. Search can be a regular expression or a custom search function written in elisp. And everything the function/re matches on is highlighted in the given theme. Emacs lisp has the advantage that identifiers of functions are global names with unique names, and the emacs lisp vm is right there. So whenever the search encours an identifier, emacs can just ask the vm: "what does this function symbol hold? Is it a marco?". The name is enough here. This does not work in Nim. An indentifier is almost never enough to even ask what that symbol means. This is very context dependent here. And communication between nimsuggest and Nim is often the cause for emacs to freeze (there is a lot of potential for optimizations).

@yuutayamada
Copy link
Contributor

I think js2-mode has own js parser of emacs-lisp and that's why they can highlight like that. I don't think we can mimic that unless someone contributes super hard. I think nimsuggest way might work (there is highlight option to query to nimsuggest, and there is a way to highlight like flycheck or flymake), but nimsuggest doesn't work if the file is too big due to that EPC (message protocol for current nimsuggest-emacs) has limit to send the data even if someone made the package for emacs.

@zetashift
Copy link

Is there anyway this is still doable? A nimsuggest way would not be possible 'cause that slows Emacs down no?

@krux02
Copy link
Contributor

krux02 commented Oct 30, 2019

@zetashift sorry for the late reply, I just get way too many notifications and sometimes they bury deep. I would not rule out a nimsuggest solution entirely, I think it can work. Emacs can handle multiple layers of highlighting with some attributes overriding others. So regular expression highlighting would be the initial fast highlighting and nimsuggest is called later to generate the correct but slow highlighting. But it should be implemented in non blocking way. The problem is, I don't know how to guarantee non blocking nimsuggest integration. Current nimsuggest integration is far from non-blocking, that is the reason I have it permanently disabled.

@zetashift
Copy link

@krux02 no problem! I kinda figured you'd be busy a lot anyway and a lot of people I know have notification floods.
I have nimsuggest disabled too because of: #227
Simple regex-based highlighting might be ideal for now. I've been wondering if it might be better to improve nimlsp and hook it up to lsp-mode or eglot.

@krux02
Copy link
Contributor

krux02 commented Nov 6, 2019

Maybe LSP is the better approach. On the other hand. I don't understand why the nimsuggest integration is so horribly complicated. I think it should be thrown away and written from scratch, but in a simpler more maintanable codebase. Unfortunately I can't do that right now.

@zetashift
Copy link

Would rewriting nimsuggest integration be better than contributing to nimlsp? I feel like rewriting nim-suggest.el might be a bit too big of a hurdle.

Also yeah time is limited :)

@krux02
Copy link
Contributor

krux02 commented Nov 6, 2019

Honestly I don't know what the better plan is. From the theoretical complexity, both emacs-nimsuggest and lsp-nimsuggest should be equal amount of complexity. And don't take current implementation as a reference for complexity. I think it is totally over engineered and over complicated. Throwing it away and writing it from scratch should be faster and simpler than trying to figure out what the current implementation does.

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

No branches or pull requests

4 participants