This is an archive of the discontinued LLVM Phabricator instance.

[include-fixer] Move cursor to #include line in vim after inserting a missing header.
ClosedPublic

Authored by hokein on Jul 14 2016, 6:40 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 63967.Jul 14 2016, 6:40 AM
hokein retitled this revision from to [include-fixer] Move curosr to #include line in vim after inserting a missing header..
hokein updated this object.
hokein added a reviewer: bkramer.
hokein added a subscriber: cfe-commits.
hokein retitled this revision from [include-fixer] Move curosr to #include line in vim after inserting a missing header. to [include-fixer] Move cursor to #include line in vim after inserting a missing header..Jul 14 2016, 6:41 AM
bkramer edited edge metadata.Jul 14 2016, 8:19 AM

I'm not entirely sure if jumping cursors are a good user interface. It will completely throw the user out of context, which is exactly what we want to avoid with include-fixer.

klimek added a subscriber: klimek.Jul 14 2016, 8:23 AM

+1 to not throwing users around by default. Can we make it configurable if folks want it?

hokein updated this revision to Diff 64004.Jul 14 2016, 10:22 AM
hokein edited edge metadata.

Add an option to make move-cursor configurable.

hokein updated this object.Jul 14 2016, 10:23 AM
hokein added a subscriber: djasper.

+Daniel who suggests this feature ;)

FWIW, I think I'd like this behavior as it shows me what is actually being done. I can undo/redo to double-check or C-O to get back to where I was. Of course making it configurable seems like the right thing to do.

include-fixer/tool/clang-include-fixer.py
102 ↗(On Diff #64004)

Seems seems very hacky. Can't you extract it out of the diff sequence above?

hokein updated this revision to Diff 64110.Jul 15 2016, 1:40 AM

Avoid a hacky way to get #include line number.

hokein marked an inline comment as done.Jul 15 2016, 1:41 AM
bkramer accepted this revision.Jul 18 2016, 2:07 AM
bkramer edited edge metadata.

lgtm now

This revision is now accepted and ready to land.Jul 18 2016, 2:07 AM
This revision was automatically updated to reflect the committed changes.