Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[python] expand comprehensions into for loops via for_in_clause #25

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alexpw
Copy link
Contributor

@alexpw alexpw commented Feb 26, 2023

This rewrites {set,list,dictionary}_ comprehensions into it's expanded for loop form. Unlike the if/else <-> ternary code, this can safely preserve all comments.

Notice: It does not support an action to toggle back to a comprehension as I haven't wrapped my head around how to approach that, except for in a very narrow case.

Warn: The most questionable thing is a decision made in order to support return statements, eg, return [x for x in range(10)]. The expanded form requires a variable to hold the list that is initialized, appended to, and returned, but there's no identifier name in this case. So, I either had to choose one, or prompt the user for one, ie with vim.fn.input(). I opted not to prompt and chose a generic/friendly name, result, instead of a generated collision-proof one like _list_accumulator_fda43. That means the comprehension's expanded form looks like:

result = []
for x in range(10):
    result.append(x)
return result

Tests include an "absurd" case to show that it supports many fors, ifs, comments, and multiline everything, everywhere, all at once.

@alexpw alexpw changed the title [python] expand comprehensions via for_in_clause [python] expand comprehensions into for loops via for_in_clause Feb 26, 2023
@CKolkey
Copy link
Owner

CKolkey commented Feb 26, 2023

Great stuff. The 'result' variable seems fine to me. Gotta put the data somewhere. I'm a little hesitant about this being a one-way transformation, but its not a dealbreaker. Whats the current issue you're facing when transforming them in the other direction?

@alexpw
Copy link
Contributor Author

alexpw commented Feb 26, 2023

Hmm, two-way is possible, but at the time it felt overwhelming. 😅 It just requires a bit of strict analysis to know if it's safe to reverse it.

  1. there must be an assignment, eg, list1 = [], defined immediately before the loop.
  2. there can be only 1 (non-for/non-if) statement in the body, it must be the deepest child, and it needs to be an append(), add(), or key=value acting on the list1 identifier.
  3. if the next sibling of the parent for loop is a simple return list1, then it will be return {comprehension} instead of an assignment.

That might actually be all of it. I'll go ahead and implement it.

@alexpw alexpw marked this pull request as draft February 26, 2023 22:11
@alexpw alexpw mentioned this pull request Feb 26, 2023
@CKolkey
Copy link
Owner

CKolkey commented Feb 27, 2023

Cool. No rush on it either - by all means, take your time to explore the solution. I'm not sure if it's of any help, but some of the modules in treesitter-refactor (https://github.com/nvim-treesitter/nvim-treesitter-refactor/blob/master/lua/nvim-treesitter-refactor/highlight_definitions.lua) might be useful to pull some semantic information out of the tree, like the variable name, or some such. Maybe, maybe not :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants