jwoudenberg / elm-pair

An artificial pair-programmer that helps you write Elm
https://elm-pair.com
GNU General Public License v3.0
109 stars 0 forks source link

Renaming false positive (import as Foo.Bar - invalid syntax) #18

Open Janiczek opened 2 years ago

Janiczek commented 2 years ago

Hello again 🙂 This is more of a discussion point, rather than a bug report.

I'm sometimes getting into situations where I start changing (ie. not renaming) a part of code (sometimes type annotations, sometimes fn usage) in a specific way that triggers elm-pair into renaming the import:

https://user-images.githubusercontent.com/149425/160117651-6062788d-2dec-408d-b3ed-45d62ba057e2.mp4

Now there are different ways to get the same result (Foo.singleton -> Bar.singleton), some of which wouldn't trigger the elm-pair change of imports. But this one is the one I've been used to befeore starting to use elm-pair 😇

The interesting thing is, it ends up in a non-compilable state (import .. as Foo.Bar is invalid syntax), which is how I realize something happened. I actually wouldn't find out if it somehow ended up with import Foo as Bar and the APIs lined up perfectly.

Should I just change my habits? Or should the list of tried elm-pair refactors be configurable, where I could eg. turn (some parts of) renaming off and keep the rest? Does the situation above have some other solution? So many questions 😅

jwoudenberg commented 2 years ago

Hi @Janiczek. I think these are great questions. I want Elm-pair to be really good at guessing your intent and doing the right thing at the right time, and these kinds of examples are super useful to improve those aspects of Elm-pair. So thank you for raising this!

I have a couple of thoughts, curious what you think!

First, Elm-pair performing a refactor that produces invalid Elm (the as Foo.Bar) I consider a bug. It'd probably be an easy fix for me, but then Elm-pair would still rename the import up to as FooBar, which wouldn't address the bigger problem.

Second thought: Elm-pair performing renames one-character-at-a-time as you type maybe looks kinda cool, but it's not an intentional design decision on my part. In fact, I think behavior might be nicer if Elm-pair waited until you were 'done' with your change, and then performed the rename in one go. Some advantages of this approach would be:

The reason I haven't implemented it this way (yet), is that I've not come up with a good heuristic for done-ness. Maybe it should be a pause in typing? Or in the case of Vim, maybe going to Normal mode? Something else?

I've also been thinking about adding a prompt/confirm button for some operations where there's a good chance Elm-pair understands programmer intent, but not good enough to act on it. The 'renaming a variable at a usage site' functionality I removed in response to #8 maybe falls in that category. I see this as complementary to 'immediate refactors' in places where intent is super clear, not as a replacement for 'immediate refactors'.

With regards to the 'changing habits' part: I've thought a bit about this before but haven't landed anywhere. If multiple easy ways to approach a change exist, would it be okay to use some as hooks for particular Elm-pair refactors and leave others for making other kinds of changes? I'm thinking in terms of Elm-pair basically having a personality, and the programmer as the other part of the pair learning how to collaborate with this 'collague' well. Is that a good experience?

I can see how configuration options would be another approach, but it's not one I'm immediately excited about. It'd add a bunch of extra functionality to test/maintain/document, and I'd much prefer to spend that time making Elm-pair do the right thing more often. I am curious though, are there specific refactors you'd turn off right not if you could?

I'll spend some more time thinking about this. If you've any additional thoughts I'd love to hear them! And thank you again for the nice example and conversation topic!

Janiczek commented 2 years ago

In fact, I think behavior might be nicer if Elm-pair waited until you were 'done' with your change, and then performed the rename in one go.

I was thinking about this vs one-char-at-a-time too! Glad to hear it doesn't seem outright impossibile to have the "larger diff" refactors to you.

The reason I haven't implemented it this way (yet), is that I've not come up with a good heuristic for done-ness. Maybe it should be a pause in typing? Or in the case of Vim, maybe going to Normal mode? Something else?

A combination of both (settle after X ms or go from insert mode to normal mode, whichever comes first) might be a good heuristic. (Also note there are ways to do stuff without ever going into insert mode 😬 pasting and such)

I'm thinking in terms of Elm-pair basically having a personality, and the programmer as the other part of the pair learning how to collaborate with this 'collague' well. Is that a good experience?

This is definitely happening for me. I'm learning to live with elm-pair, getting more aware about which "motions" are problematic. It would definitely be a better experience if there were no problematic motions, but I don't know how realistic that is.

I am curious though, are there specific refactors you'd turn off right not if you could?

I believe the renaming has been the most problematic so far. The binding name renamings were mostly fixed in previous releases, and the only thing currently remaining and giving me an ocassional trouble are the renamings that touch imports. And again, that's mostly because I go:

instead of removing the original Foo first and then writing the Bar replacement.

I'm not sure how you organize your refactors but I'd be trying to turn off some of those that get triggered by the above behaviour.

I think I understand your hesitance towards config files BTW 👍

jwoudenberg commented 2 years ago

Thank you for sharing your thoughts!

I want to experiment a bit with larger diff refactors, but am going to finish the in-progress support for cross-file renames first. To be continued! I'll keep this issue open in the meanwhile.