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

Fix undo and partial format #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix undo and partial format #61

wants to merge 1 commit into from

Conversation

Ram-Z
Copy link

@Ram-Z Ram-Z commented Apr 10, 2017

This should fix the undo issue as well as improve partial formatting.

It works by sending only the selected lines to clang-format to be formatted, instead of the entire file. We then remember the indentation (because it gets lost during format) and reapply it to the formatted selection.

This was originally committed on Ram-Z@19b4346, but a change in circumstances led me to forget about it since. I've rebased that commit onto the current master. Sorry for the delay.

Fix: #8

@Ram-Z
Copy link
Author

Ram-Z commented Apr 10, 2017

Didn't realize there were tests.

So... clang_format#format(line1, line2) now only returns the part between line1 and line2 which has been formatted. This is not what the tests expect. I'll have a quick look at fixing these.

Ram-Z added a commit to Ram-Z/vim-clang-format that referenced this pull request Apr 10, 2017
This fixes some of the tests by limiting the output of the ClangFormat
helper function to between `line1` and `line2`. This is similar to what
`clang_format#format` now returns.

There are two problems left:
1. the number of lines returned by `clang_format#format` might not be
   equal to `line2 - line1`, i.e when it splits a line.
2. `clang_format#replace` now handles reidenting the lines correctly, so
   any tests that format a line that is indented will fail.

Ref: rhysd#61
@Ram-Z
Copy link
Author

Ram-Z commented Apr 10, 2017

I've spent more time than I care to on this now. See the last commit msg for details on what is failing.

I'm happy to have another look at fixing the tests after I know that you are interested in merging this in.

@Ram-Z
Copy link
Author

Ram-Z commented May 10, 2017

@rhysd Any comments on this?

@rhysd
Copy link
Owner

rhysd commented May 11, 2017

As long as looking code, this would break the code when formatting only line 4 of below code

#include <iostream>
int main()
{
    std::cout << "Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the \"Software\"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:";
}

I think slicing the formatted code does not work.

@Ram-Z
Copy link
Author

Ram-Z commented May 11, 2017

OK... I made some wrong assumption about how > would work.

With this code the above does work:

let indent = indent(a:line1)
silent exe a:line1.','a:line2.'delete _'

call setreg('g', formatted, 'V')
silent put! g
silent exe "'[,']s/^/".repeat(' ', indent)."/"

but it has other problems when noexpandtab is set for example, nothing that can't be worked around.

The problem with replacing the entire file is that you are always replacing everything in the file even though you only format a single line. I think that slicing is the way to go. I would however prefer if clang-format would simply keep the starting indentation of the snippet when it formats it.

Ram-Z added a commit to Ram-Z/vim-clang-format that referenced this pull request Oct 4, 2017
This fixes some of the tests by limiting the output of the ClangFormat
helper function to between `line1` and `line2`. This is similar to what
`clang_format#format` now returns.

There are two problems left:
1. the number of lines returned by `clang_format#format` might not be
   equal to `line2 - line1`, i.e when it splits a line.
2. `clang_format#replace` now handles reidenting the lines correctly, so
   any tests that format a line that is indented will fail.

Ref: rhysd#61
Ram-Z added a commit to Ram-Z/vim-clang-format that referenced this pull request Oct 7, 2017
This fixes some of the tests by limiting the output of the ClangFormat
helper function to between `line1` and `line2`. This is similar to what
`clang_format#format` now returns.

There are two problems left:
1. the number of lines returned by `clang_format#format` might not be
   equal to `line2 - line1`, i.e when it splits a line.
2. `clang_format#replace` now handles reidenting the lines correctly, so
   any tests that format a line that is indented will fail.

Ref: rhysd#61
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.

Cursor position not maintained after undo
2 participants