fredrikekre/Runic.jl

[Feature Request] Add noindent annotation

Opened this issue · 3 comments

fredrikekre commented

I am a bit hesitant to add something like this. It seems like a very niche usecase if applying a macro to a complete file is the main motivation.

I would find it important to know that the code I am reading is inside a macro. In the linked package it would be impossible to tell that what I am reading will be transformed with a macro from 100s if not 1000s of lines above. The indent that Runic adds at least hints to that the code I see is inside something. To me it would be so much clearer to apply the macro locally instead. I looked at one of the files (where you commented) and it was something like 700 lines of code and only 4 or 5 places where the macro would have an affect. Perhaps it isn't my place to comment on this style, but to me it seems strange to blindly wrap a whole just for the macro to be usefule in a few places.

Would you consider rewriting to use it locally? Alternatively apply it on the function level (like e.g. @inline) or use the include(mapexpr, filename) API?

  • 👍1
  • 👀1
ranocha commented

Would you consider rewriting to use it locally? Alternatively apply it on the function level (like e.g. @inline)

We had several discussions about this. One range of thoughts argued like you (and like I would prefer in several places for code clarity), and another line of thought wanted to keep most parts of the code as "clean" as possible to make it easier to read for students working on the mathematics and not the CS aspects. I don't want to re-init this discussion right now.

or use the include(mapexpr, filename) API?

This does not work well with Reevise.jl, see timholy/Revise.jl#634

KristofferC commented

My opinion is that putting the macro close to the use sites is clearly correct and it is good that Runic indents it to make the code clearer that an (arbitrary) transformation of the code is taking place.