mako icon indicating copy to clipboard operation
mako copied to clipboard

New feature: indenting the text in "context.write()" relative to the indentation of the entry of a python code block ("<%")

Open rdan opened this issue 3 months ago • 2 comments

I implemented this feature differently from the code you proposed. The main goal was to be able to retrieve for any "context.write()" in a code block the indentation of "<%". To achieve that, I added a new action in "codegen" which is to get the current position of the last element in the buffer "data" in the current "FastEncodingBuffer". I think that this operation is simple enough to not have an impact on the global performance of the tool even if this new feature is not used.

I tried to minimize the modifications of "FastEncodingBuffer" so I just added a method to get the length of data and I added a parameter of "getvalue()" to be able to use this position while concatenating the strings. Due to this choice, each time "context.write()" is called with the new parameter equal to True, this concatenation on almost the full content of data will happen twice. I did some performance tests on different strategies to search for the last "\n" in data and the concatenation can be from 2 times to 14 times slower than doing a search loop on data. If you think that the gain of performance is worth implementing a new method in "FastEncodingBuffer" (I was thinking about something like "get_text_since_last_cr(self, pos=None)" which will return a string without "\n" starting from the end of "data" or from "data[pos-1]") and that this new method makes sense in "FastEncodingBuffer", then I can do an update.

Regarding the added tests, do you see any additional tests that I should implement ?

And regarding the non-regression tests, do you want me to also run the 53 skipped tests ? How can I run them ? What do I need to install ?

Also, if I made any mistakes in the process of development on github, please let me know.

Modifications:

util.py::FastEncodingBuffer

  • deque replaced by list (no specific deque feature used, list faster than deque, deque does not allow slicing which is used by the new feature)
  • Creation of method "getpos()" to get the current location in "data"
  • Update of method "getvalue()" to get either the full content of "data" or only the content of data up to the position retrieved previously by "getpos()"

codegen.py::_GenerateRenderMethod::visitCode()

  • Mark the current location in the current FastEncodingBuffer

runtime.py::Context

  • Creation of the method "code_block_entry_mark()" that marks the current location in the current FastEncodingBuffer
  • Update of the method "write()"
    • New parameter "indent_relative_to_code_block_entry" allowing to activate the new feature
    • Implementation of the new feature

New tests added in test_util.py & test_runtime.py to cover the updates described above

466 tests passed, 53 skipped (linked to babel, lingua, Beaker, dogpile)

rdan avatar Sep 21 '25 17:09 rdan

Thank you for the review. I understand your answer. I just tried the code you provided me with an updated version of "test_runtime.py" (test_runtime.py) including the 3 use cases I provided in the Discussion (these are the last 3 tests of "test_runtime.py"). On the 6 tests, test 2 is OK but only this is just a side effect, tests 1 & 3 cannot work because I added a new use case to this feature (each "\n" except if it is the last character in the string written using "context.write(string, indent_current=True)" is replaced by "\n" + indent, like this, everything written by "context.write()" is indented relatively to "<%"). And the tests 4 to 6 are failing also because each "context.write()", I think, is not relying on the indentation of "<%" but on the indentation of the text written by the previous "context.write()" in the code block (and, on "<%" for the first "context.write()" in the code block). And this is not what I need. I am not sure if a non intrusive solution exists to implement this feature.

rdan avatar Sep 21 '25 20:09 rdan

also I forgot to mention the main thing I dont want to do is add another function call to all the python blocks, so the change in codegen that adds two function calls (the outer function and the len()) is the biggest problem. The custom block approach I have above has the advantage that we can grab that preceding indent right as our block begins and then we have it within the block.

zzzeek avatar Sep 22 '25 03:09 zzzeek