Skip to content

Fixing vim's handling of the enclosing-type cache - #771

Merged
trefis merged 1 commit into
ocaml:masterfrom
ELLIOTTCABLE:fixing-vim-type-enclosing
May 23, 2018
Merged

Fixing vim's handling of the enclosing-type cache#771
trefis merged 1 commit into
ocaml:masterfrom
ELLIOTTCABLE:fixing-vim-type-enclosing

Conversation

@ELLIOTTCABLE

@ELLIOTTCABLE ELLIOTTCABLE commented Jan 4, 2018

Copy link
Copy Markdown
Contributor

The usage of the enclosing_types cache in the Vim plugin is pretty bugged — fixes #732, #770.

This is my first PR, so I intentionally went a bit overboard with explanatory comments; I also tried to match the code-style while moving things around.

I won't be offended at all if you have any feedback on how you want this cleaned up — feel free to throw any requirements at me if you want this massaged! <3

@ELLIOTTCABLE

Copy link
Copy Markdown
Contributor Author

Anything blocking this? (=

@trefis

trefis commented May 14, 2018

Copy link
Copy Markdown
Contributor

Anything blocking this?

Yes, the lack of people available for reviewing :p

So, I just had a look, and it took me a while to see what the actual change was, that's because the diff is bigger than it needs to: why did you inline the function instead of living in on its own?
If you hadn't, the change would have been more obvious.

Apart from that nitpick I think I understand and agree with your change.

If you can undo the inlining and squash all your commits then I'll happily merge this.

@ELLIOTTCABLE

ELLIOTTCABLE commented May 14, 2018

Copy link
Copy Markdown
Contributor Author

@trefis hah! turns out my Python was just shaky. I needed access to enclosing_types[0] for the fix; I assumed it was local, didn't notice it was global, saw it was only used in one place, and took the expedient action. ‘Outlined!’

Squashing, rebasing, and pushing now. <3

@ELLIOTTCABLE
ELLIOTTCABLE force-pushed the fixing-vim-type-enclosing branch from 9b80dd7 to bcc3b85 Compare May 14, 2018 20:26
@ELLIOTTCABLE
ELLIOTTCABLE force-pushed the fixing-vim-type-enclosing branch from bcc3b85 to f1403a0 Compare May 14, 2018 20:26
@trefis
trefis merged commit 5b69fe4 into ocaml:master May 23, 2018
@trefis

trefis commented May 23, 2018

Copy link
Copy Markdown
Contributor

Thank you!

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.

MerlinGrowEnclosing / LL-n explodes if I attempt to grow the selection too much

2 participants