OvermindDL1 / protocol_ex

Elixir Extended Protocol
https://hex.pm/packages/protocol_ex
49 stars 5 forks source link

ExDoc now uses makedown #1

Closed tmbb closed 7 years ago

tmbb commented 7 years ago

Now with makeup

OvermindDL1 commented 7 years ago

Added a comment. Also I'm curious, why is the css and such included in 'this' applications priv (meaning it will be pulled in as libraries) instead of pulling it from makeup's priv instead? That is a lot of weight in priv here when it is not needed (unless overriding?)? :-)

tmbb commented 7 years ago

I don't know, never really thought about it... I think it's more flexible to have that in the application's priv, because you can only specify a single assets directory. That way, if you want to add some extra CSS (beyond the one makedown) adds you'd be in trouble.

Is this a showstopper for you?

OvermindDL1 commented 7 years ago

I don't know, never really thought about it... I think it's more flexible to have that in the application's priv, because you can only specify a single assets directory. That way, if you want to add some extra CSS (beyond the one makedown) adds you'd be in trouble.

I'd personally have it fallback to makeup's own css directory by default. If someone wants to customize it then they can copy it over elsewhere and use it there. But having the documentation CSS by distributed with the library itself is a lot of additional noise for no gain in distributions. ^.^;

Is this a showstopper for you?

Hmm, unsure, I like clean filesystems... >.>

Isn't it possible to just change assets: "priv/doc/assets", to point to assets: "deps/makedown/priv/doc/assets", or so while not including the css files directly?

tmbb commented 7 years ago

Isn't it possible to just change assets: "priv/doc/assets", to point to assets: "deps/makedown/priv/doc/assets", or so while not including the css files directly?

Yes, that should work. I haven't time to try it now, though.

OvermindDL1 commented 7 years ago

Whooo it's so slim now! :-)

Ready for merging it seems, everything good to or are you still testing? :-)

tmbb commented 7 years ago

I didn't run any tests. Just generated the docs. You can build those locally and see for yourself. Everything's fine with the docs.

tmbb commented 7 years ago

Just a reminder: the function names in the function definitions are NOT highlighted by makeup. Please see if you're happy with the new look before merging!

OvermindDL1 commented 7 years ago

Just a reminder: the function names in the function definitions are NOT highlighted by makeup. Please see if you're happy with the new look before merging!

Highlighting them would be very nice. Have you thought of adding an option that I could add to the mix.exs or config.exs or so file to state what special highlighting should be done, like marking defprotocolex as a def-like keyword or so?

tmbb commented 7 years ago

Highlighting them would be very nice

I'd have to add a new defrule (or more than one) but it's certainly doable. The main complication is dealing with operator definitions which might require some hardcore parsing of the function definition so that it doesn't color things inappropriately. For example:

def a + b, do: a - b # 1
def a(x), do: x # 2

In the first line, the a shouldn't be highlighted, and in the second one it should be highlighted. EDIT: as you can see, GitHub's highlighter does NOT highlight this correctly. EDIT2: Also, Visual Studio Code does NOT highlight this correctly either.

Also, I must handle this intelligently:

# inside a macro
def unquote(funcname)(x), do: x

I want to highlight unquote as a keyword/special form and not as a function name. EDIT: GitHub's highlighter does this "wrong", for example.

Have you thought of adding an option that I could add to the mix.exs or config.exs or so file to state what special highlighting should be done, like marking defprotocolex as a def-like keyword or so?

Yes. This is actually much easier given my implementation. Highlighting keywords is a second pass after the parsing, and I can inject user-defined keywords into that. I'm thinking of something like this:

config :makeup,
  extra_keywords: [
    elixir: [
      def_like: ["defprotocolex", "defimplex", "defthis", "defthat"]
    ],
    common_lisp: [
      def_like: ["def-my-crazy-macro", "def-my-crazier-macro", "def-super-krazy-makro"]
    ],
    rust: [
      def_like: ["def_type_level_sudoku_solver"]
    ]
  ]

(obviously, Makeup doesn't support common lisp or rust yet)

tmbb commented 7 years ago

Makeup (my local repo, not on GitHub yet) already highlights the function names in the function heads but I need some time to test some edge cases. Might take a couple of days.

EDIT: example:

image

tmbb commented 7 years ago

I've done some radical changes in architecture. Makeup now comes with no lexers by default, and lexers are extra packages that must be imported. This saves a lot of time because you only compile the lexers you use. I'll create a new package (makeup_bundle or something like that which will include all lexers by default).

OvermindDL1 commented 7 years ago

I've done some radical changes in architecture. Makeup now comes with no lexers by default, and lexers are extra packages that must be imported. This saves a lot of time because you only compile the lexers you use. I'll create a new package (makeup_bundle or something like that which will include all lexers by default).

Awesome, definitely better to only bring in what your project needs. :-)

tmbb commented 7 years ago

Everything should be now highlighted correctly. For example:

image

In the screenshot above:

Still no support for custom def-like keywords.

I think you can merge this now.

OvermindDL1 commented 7 years ago

Hah, that looks awesome! :-)

The only special forms I'd highlight in function name position would probably be just unquote as I don't think anything else there makes sense? Though I could probably come up with valid code if I tried... ^.^;

Merging! Wootness!!

I await custom keyword support (make sure to only highlight them as such in call positions, not name positions ^.^). :-)

tmbb commented 7 years ago

I don't think anything else there makes sense?

I've been finding Elixir too surprising (not necessarily in a good way) to decide for myself what makes sense and what doesn't xD

I await custom keyword support

I was thinking of adding a configurable list of def-like keywords. They would only be active when actually defining something.

(make sure to only highlight them as such in call positions, not name positions ^.^)

Hm... I don't know... That would be different from the way "real" keyword work, right? A real keyword can't be a variable. Could you give a concrete example of something you have in mind?

OvermindDL1 commented 7 years ago

I've been finding Elixir too surprising (not necessarily in a good way) to decide for myself what makes sense and what doesn't xD

Lol, I entirely agree. The only way to know for sure is to not only parse it but also 'run' the module construction step (up to but not including building the BEAM). ^.^;

I was thinking of adding a configurable list of def-like keywords. They would only be active when actually defining something.

Hmm, an example?

Hm... I don't know... That would be different from the way "real" keyword work, right? A real keyword can't be a variable. Could you give a concrete example of something you have in mind?

Well if I want defprotocol_ex highlighted I would want it highlighted when doing defprotocol_ex blah and defprotocol_ex(blah) and &defprotocol_ex/1 and so forth, but not when it is defprotocol_ex = 42 or def defprotocol_ex(body) do ... end and such. :-)

tmbb commented 7 years ago

Well if I want defprotocol_ex highlighted I would want it highlighted when doing defprotocol_ex blah and defprotocol_ex(blah) and &defprotocol_ex/1 and so forth, but not when it is defprotocol_ex = 42 or def defprotocol_ex(body) do ... end and such. :-)

This is probably never going to happen with the current architecture. I'd have to parse pattern matches and stuff like that...

I might do it when I manage to hack into the juicy introspection tools that the BEAM provides, but it's not on the roadmap right now.

tmbb commented 7 years ago

Hmm, an example?

An example for ExActor:

defcall mycall(x), do: nil

I've just noticed that the lexer doesn't work for defprotocol_ex, because it's followed by a module name and not an identifier. But that's the idea. Ideally, both defcall and defprotocol_ex would be highlighted the same way as def or defmacro.

OvermindDL1 commented 7 years ago

True true yep. :-)

tmbb commented 7 years ago

With my private fork of ex_doc (and my private ex_spirit fork with the bug fixed), makeup can do this:

image

defimplEx declarations are highlighted and deftest not only is highlighted but the function name is highlighted as a function name too.

EDIT: the ex_doc fork contains some chnages that are not yet in the PR, so that you can do this:

     markdown_processor_options: [
          lexer_options: %{
            "elixir" => [
              extra_declarations: [
                "defimplEx", "defimpl_ex",
                "defprotocolEx", "defprotocol_ex"],
              extra_def_like: ["deftest"]]
          }
        ]
OvermindDL1 commented 7 years ago

Ooooo, that is awesome!