shrinko8 icon indicating copy to clipboard operation
shrinko8 copied to clipboard

Add function token count comments (rough draft)

Open sparr opened this issue 2 years ago • 6 comments

This needs a lot of work, but the core functionality is working so I'm submitting this for feedback on the implementation so far.

TODO list:

  • replace annotation comment instead of adding another one each time
  • test combined with other shrinko8 functionality, e.g. minify

Stretch goals:

  • additional options
    • don't add annotation comments for nested functions
    • exclude nested functions from parent function counts
    • put annotation comment above existing non-annotation comments

The --annotate option turns this:

-- leading comment
-- comment above func1
function func1(a,b)
 c={a,b,};
 return c[1]
end
-- comment above func2
function func2()
 return true
end
-- trailing comment

into this:

-- leading comment
-- comment above func1
-- T:14 C:47 minC:36
-- minified source:
-- function u(t,r)n={t,r}return n[1]end
function func1(a,b)
 c={a,b,};
 return c[1]
end
-- comment above func2
-- T:5 C:33 minC:27
-- minified source:
-- function n()return true end
function func2()
 return true
end
-- trailing comment

Closes #12

sparr avatar May 28 '23 00:05 sparr

Updated top comment with current status. Still very much WIP.

Found a failure mode that I couldn't immediately resolve. This cart:

local x = function() end
function y() x() end

Produces this error:

Traceback (most recent call last):
  File "/home/sparr/src/shrinko8/shrinko8.py", line 216, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/home/sparr/src/shrinko8/shrinko8.py", line 182, in main
    ok, errors = process_code(ctxt, src, input_count=args.input_count, count=args.count,
  File "/home/sparr/src/shrinko8/pico_process.py", line 169, in process_code
    annotate_code(ctxt, source, root)
  File "/home/sparr/src/shrinko8/pico_annotate.py", line 28, in annotate_code
    apply_node_tree(root, comment_before_function)
  File "/home/sparr/src/shrinko8/pico_annotate.py", line 9, in apply_node_tree
    apply_node_tree(child, func)
  File "/home/sparr/src/shrinko8/pico_annotate.py", line 10, in apply_node_tree
    func(node)
  File "/home/sparr/src/shrinko8/pico_annotate.py", line 22, in comment_before_function
    rename_tokens(ctxt, node, True)
  File "/home/sparr/src/shrinko8/pico_rename.py", line 272, in rename_tokens
    if ident_global in var.scope.used_globals:
AttributeError: 'Scope' object has no attribute 'used_globals'

This is weird because it's happening when pico_annotate asks pico_rename to rename symbols in function y() x() end where it shouldn't be aware of the local x = function() end above, but removing the local definition avoids the error. My initial suspicion is that I've made a mistake in using a single PicoContext for all my operations.

sparr avatar May 28 '23 14:05 sparr

This is because pico_rename adds a few attributes (like used_globals) to the scopes it receives and assumes that all scopes then have it. Why do you need to run pico_rename on each function independently? It's unlikely to give sensible results anyway - you should run it on the entire thing.

thisismypassport avatar May 30 '23 01:05 thisismypassport

Running it on the whole thing will use two letter identifiers for files with more than 26 globals (or outer scope locals), while doing one function at a time will use single characters almost always. I'll try to pay more attention to when the scopes get those new attributes and when they are checked for.

sparr avatar May 30 '23 04:05 sparr

It'd probably make sense to move the used_globals/locals definitions to the Scope class itself (its init) instead of monkey patching it - should resolve this exception.

thisismypassport avatar May 30 '23 22:05 thisismypassport

I moved used_locals/globals (& now a very-rarely-needed used_members) to be lazy properties on Scope now, FYI.

thisismypassport avatar Jun 23 '23 13:06 thisismypassport

Thanks! That resolved the problems I was having. I think the core functionality here is working now.

The next thing I need to do is get it to recognize its own comments so it will overwrite them and be idempotent instead of adding more new comments each time.

And then there's discussion about how to expose it with new options. I'm thinking of one option that adds the -- T:9 C:86 minC:29 annotation, and another that also adds the minified source comment.

sparr avatar Jun 23 '23 23:06 sparr