Skip to content

Conversation

rranelli
Copy link
Contributor

This PR sets module fontlock to be consistent across references. This PR also removes the notion of highlighting built-in modules differently than user/custom modules.

Concerning the risk of wrongly highlighting capitalized words that do not correspond to module references, the picture below sumarizes the changes in highlighting:

screenshot from 2015-03-18 23 37 37

To the left, the current behavior and to the right the proposed ones.

UPDATE:
I fontified the contents of sigils with string-font-lock-face just like the regexp does.

The new result is as follows:

screenshot from 2015-03-20 20 12 35

Fixes #166

@rranelli rranelli changed the title Highlight module references equally [wip] Highlight module references equally Mar 19, 2015
@rranelli rranelli force-pushed the highlight-all-modules-equally branch from 077c5bb to 458d12a Compare March 19, 2015 02:39
@rranelli
Copy link
Contributor Author

@mattdeboard @tonini @whatyouhide

I've fixed the things I've left pending before, I think this is ok to be properly reviewed now.

@rranelli rranelli changed the title [wip] Highlight module references equally Highlight module references equally Mar 20, 2015
@rranelli rranelli force-pushed the highlight-all-modules-equally branch from 5f47d67 to 75d4fb3 Compare March 20, 2015 23:15
@rranelli
Copy link
Contributor Author

ping,

Any thoughts on this PR ?

Thanks!

@mattdeboard
Copy link
Contributor

Hi @rranelli

Sorry about the unresponsiveness, I know it's frustrating.

Can you sort out merge conflicts here please?

@rranelli
Copy link
Contributor Author

@mattdeboard No problem at all =).

I will sort the merge conflicts right now.

@rranelli rranelli force-pushed the highlight-all-modules-equally branch from 75d4fb3 to 8d1944f Compare May 20, 2015 00:09
@rranelli
Copy link
Contributor Author

Done

@@ -330,6 +305,33 @@ for the Elixir programming language."
(,(elixir-rx (group sigils))
1 font-lock-builtin-face)

;; Sigil patterns. Elixir has support for eight different sigil delimiters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you modified this correctly but you left these lines in tact. Please remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasnt sure if sigils and regexes should be highlighted differently.

Should they look the same then ?

@mattdeboard
Copy link
Contributor

Also I just saw that ~s is also included in the heredoc regex pattern. Is this why ~r and ~s are highlighted different colors in your screenshot?

@mattdeboard
Copy link
Contributor

To answer my own question, yes it is. Super minor issue though, not going to worry about it.

@rranelli rranelli force-pushed the highlight-all-modules-equally branch from 8d1944f to 60047aa Compare May 20, 2015 04:33
@mattdeboard
Copy link
Contributor

Fixes #166

mattdeboard added a commit that referenced this pull request May 20, 2015
Modify syntax highlighting so there is no differentiating between built-in and user-defined modules.
@mattdeboard mattdeboard merged commit 531a2b3 into elixir-editors:master May 20, 2015
@rranelli rranelli deleted the highlight-all-modules-equally branch November 24, 2015 19:52
J3RN pushed a commit to J3RN/emacs-elixir that referenced this pull request Apr 24, 2021
…-editors#179)

* DocumentSymbols provider should handle modules without a name

The DocumentSymbols provider should be able to handle a variety of compilation
errors so that it can be useful while debugging the compilation errors. The main
problem was that the `extract_symbol/2` clause that handled `defmodule` and
`defprotocol` was overly restrictive in the pattern match that it did. So I've
changed it to match all the possible AST variations (although some may still
raise an exception within the clause, but this prevents bad data from
propagating).

`MISSING_MODULE_NAME` was returned since a non-empty name is required

* Update test to use == on an assert that is not making use of pattern matching

* Update changelog
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.

Highlighting of built-in vs. user-defined modules
2 participants