Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein added a comment. In http://reviews.llvm.org/D20621#444050, @bkramer wrote: > LG. Can't wait to use it myself :) Currently, the header is only inserted at the first line of the file because we don't output the FirstIncludeOffset to py script. A follow-up patch will come soon. Repository: rL LLVM http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL271258: [include-fixer] Create a mode in vim integration to
show multiple potential… (authored by hokein).
Changed prior to commit:
http://reviews.llvm.org/D20621?vs=59028=59030#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20621
Files:
clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/IncludeFixer.h
clang-tools-extra/trunk/include-fixer/IncludeFixerContext.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
clang-tools-extra/trunk/test/include-fixer/commandline_options.cpp
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
Index: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
===
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.h
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.h
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h
@@ -10,7 +10,9 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H
#define LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_INCLUDEFIXER_H
+#include "IncludeFixerContext.h"
#include "SymbolIndexManager.h"
+#include "clang/Format/Format.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Tooling.h"
#include
@@ -28,13 +30,12 @@
class IncludeFixerActionFactory : public clang::tooling::ToolAction {
public:
/// \param SymbolIndexMgr A source for matching symbols to header files.
- /// \param Replacements Storage for the output of the fixer.
+ /// \param Context A context for the symbol being queried.
/// \param StyleName Fallback style for reformatting.
/// \param MinimizeIncludePaths whether inserted include paths are optimized.
- IncludeFixerActionFactory(
- SymbolIndexManager , std::set ,
- std::vector ,
- StringRef StyleName, bool MinimizeIncludePaths = true);
+ IncludeFixerActionFactory(SymbolIndexManager ,
+IncludeFixerContext , StringRef StyleName,
+bool MinimizeIncludePaths = true);
~IncludeFixerActionFactory() override;
@@ -48,11 +49,8 @@
/// The client to use to find cross-references.
SymbolIndexManager
- /// Headers to be added.
- std::set
-
- /// Replacements are written here.
- std::vector
+ /// The context that contains all information about the symbol being queried.
+ IncludeFixerContext
/// Whether inserted include paths should be optimized.
bool MinimizeIncludePaths;
@@ -62,6 +60,25 @@
std::string FallbackStyle;
};
+/// Create replacements for the header being inserted. The replacements will
+/// insert a header before the first #include in \p Code, and sort all headers
+/// with the given clang-format style.
+///
+/// \param Code The source code.
+/// \param FilePath The source file path.
+/// \param Header The header being inserted.
+/// \param FirstIncludeOffset The insertion point for new include directives.
+/// The default value -1U means inserting the header at the first line, and if
+/// there is no #include block, it will create a header block by inserting a
+/// newline.
+/// \param Style clang-format style being used.
+///
+/// \return Replacements for inserting and sorting headers.
+std::vector createInsertHeaderReplacements(
+StringRef Code, StringRef FilePath, StringRef Header,
+unsigned FirstIncludeOffset = -1U,
+const clang::format::FormatStyle = clang::format::getLLVMStyle());
+
} // namespace include_fixer
} // namespace clang
Index: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
@@
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
klimek accepted this revision. klimek added a reviewer: klimek. klimek added a comment. lg http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein marked an inline comment as done. hokein added a comment. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 59028.
hokein added a comment.
Add -1U comment back.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+print "Added #include {0} for {1}.\n".format(lines[1],
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
klimek added inline comments. Comment at: include-fixer/IncludeFixer.h:74 @@ +73,3 @@ +/// \return Replacements for inserting and sorting headers. +std::vector createInsertHeaderReplacements( +StringRef Code, StringRef FilePath, StringRef Header, This is still missing documentation on what the -1U special case means. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 59026.
hokein added a comment.
Update a out-of-date comment.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+print "Added #include {0} for
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. LG. Can't wait to use it myself :) http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 59018.
hokein marked 2 inline comments as done.
hokein added a comment.
Fix code style.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+print "Added
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
bkramer added inline comments. Comment at: include-fixer/IncludeFixer.h:71 @@ +70,3 @@ +/// \param FirstIncludeOffset The insertion point for new include directives. +/// -1U(UINT_MAX) indicates uninitialization, which will insert the header at +/// first line. If there is no #include block in the Code, it will create a I think you can drop the part about 'uninitialization' from this comment. Comment at: include-fixer/IncludeFixer.h:79 @@ +78,3 @@ +StringRef Code, StringRef FilePath, StringRef Header, +unsigned FirstIncludeOffset=-1U, +const clang::format::FormatStyle =clang::format::getLLVMStyle()); This doesn't look clang-formatted :p http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein added inline comments. Comment at: include-fixer/IncludeFixer.h:80 @@ +79,3 @@ +unsigned FirstIncludeOffset=-1U, +const clang::format::FormatStyle =clang::format::getLLVMStyle()); + Using a default argument in `Style` can simplify the code in some cases, for example in the Unittest. @klimek might have idea? http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58964.
hokein marked an inline comment as done.
hokein added a comment.
Use format::getStyle to get clang-format style.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1],
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
ioeric added inline comments.
Comment at: include-fixer/IncludeFixer.h:80
@@ +79,3 @@
+unsigned FirstIncludeOffset=-1U,
+const clang::format::FormatStyle =clang::format::getLLVMStyle());
+
I don't see why we'd want Style to be optional.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:110
@@ +109,3 @@
+ format::FormatStyle InsertStyle;
+ format::getPredefinedStyle(Style, format::FormatStyle::LK_Cpp, );
+
We might still want to use `clang::format::getStyle("file", FilePath,
FallbackStyle);`, which will use format style defined in clang-format config
file in the path, if such config file exists. `Style` option is just a fallback
style.
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58955.
hokein marked an inline comment as done.
hokein added a comment.
Refactor createReplacementsForHeaders.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createInsertHeaderReplacements(
+Code, "input.cc", FixerContext.Headers.front(),
+FixerContext.FirstIncludeOffset);
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
klimek added a subscriber: klimek.
Comment at: include-fixer/IncludeFixer.cpp:397
@@ +396,3 @@
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+// FIXME: skip header guards.
Do we want UINT_MAX? I find the wording in the standard to be too involved for
me to easily understand that this is both well-defined and what we want and
portable.
Comment at: include-fixer/IncludeFixer.h:62-72
@@ -64,1 +61,13 @@
+/// Insert a header before the first #include in \p Code and run
+/// clang-format to sort all headers.
+/// \param Code The source code.
+/// \param FilePath The source file path.
+/// \param StyleName Fallback style for reformatting.
+/// \param FirstIncludeOffset The insertion point for new include directives.
+/// \param Header The header being inserted.
+/// \return Replacements for inserting and sorting headers.
+std::vector createReplacementsForHeader(
+StringRef Code, StringRef FilePath, StringRef FallbackStyle,
+unsigned FirstIncludeOffset, StringRef Header);
+
1. This only needs one style, so I think we should pass it in instead of
FilePath and FallbackStyle
2. From the docs it seems like FirstIncludeOffset should always be INT_MAX? Can
we just leave out that parameter and make the function do the right thing?
3. Generally, I'd order parameters by "importance", that is, the ones that are
core to the functionality go first (C++ kinda implies this by only allowing
later parameters being defaultable); so basically, I'd make this:
.. insertHeader(StringRef Code, StringRef Header, const FormatStyle& Style);
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58942.
hokein added a comment.
Remove unneeded headers.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createReplacementsForHeader(
+Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+print "Added #include {0} for
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58941.
hokein marked 9 inline comments as done.
hokein added a comment.
Address comments.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
test/include-fixer/commandline_options.cpp
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::createReplacementsForHeader(
+Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: test/include-fixer/commandline_options.cpp
===
--- /dev/null
+++ test/include-fixer/commandline_options.cpp
@@ -0,0 +1,12 @@
+// REQUIRES: shell
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -output-headers %t.cpp -- | FileCheck %s -check-prefix=CHECK-HEADERS
+// RUN: cat %t.cpp | clang-include-fixer -stdin -insert-header='"foo.h"' %t.cpp | FileCheck %s -check-prefix=CHECK
+//
+// CHECK-HEADERS: "foo.h"
+// CHECK-HEADERS: "bar.h"
+//
+// CHECK: #include "foo.h"
+// CHECK: foo f;
+
+foo f;
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -18,7 +18,6 @@
import argparse
import difflib
import subprocess
-import sys
import vim
# set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
@@ -28,6 +27,39 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+maximum_suggested_headers=3
+if vim.eval('exists("g:clang_include_fixer_maximum_suggested_headers")') == "1":
+ maximum_suggested_headers = max(
+ 1,
+ vim.eval('g:clang_include_fixer_maximum_suggested_headers'))
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
+def InsertHeaderToVimBuffer(header, text):
+ command = [binary, "-stdin", "-insert-header="+header,
+ vim.current.buffer.name]
+ stdout, stderr = execute(command, text)
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +73,36 @@
buf = vim.current.buffer
text = '\n'.join(buf)
- # Call clang-include-fixer.
- command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
+ # Run command to get all headers.
+ command = [binary, "-stdin", "-output-headers", "-db="+args.db, "-input="+args.input, "-debug",
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
+ lines = stdout.splitlines()
+ if len(lines) < 2:
+print "No header is included.\n"
+return
- # If successful, replace buffer contents.
- if stderr:
-print stderr
+ # The first line is the symbol name.
+ symbol = lines[0]
+ # If there is only one suggested header, insert it directly.
+ if len(lines) == 2 or maximum_suggested_headers == 1:
+InsertHeaderToVimBuffer(lines[1], text)
+print "Added
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein added a comment. In http://reviews.llvm.org/D20621#439447, @bkramer wrote: > Can you add some lit tests for the various command line modes > clang-include-fixer has now. We can't reasonably test the vim integration but > we can tests the bits it's composed of. Done. Comment at: include-fixer/tool/clang-include-fixer.py:49 @@ +48,3 @@ + default_choice_index) + return int(vim.eval(to_eval)); + ioeric wrote: > Maybe handle cases where `to_eval` is not a valid index? vim can handle this for us. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
bkramer added inline comments. Comment at: include-fixer/tool/clang-include-fixer.py:84 @@ +83,3 @@ +index = 1; +for header in lines[1:]: + choices_message += "&" + str(index) + header + "\n" ioeric wrote: > If there is only one candidate, it doesn't make sense to ask users to choose > it IMO. We can simply insert the header in this case. > > And I think we should make users pick the correct header if there are > multiple candidates by default. What do you think, Ben? @bkramer I agree, we should ask by default when there is more than one suggested header. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
ioeric added inline comments.
Comment at: include-fixer/IncludeFixer.cpp:241
@@ +240,3 @@
+ IncludeFixerContext
+ GetIncludeFixerContext(const clang::SourceManager ,
+ clang::HeaderSearch ) {
I think function name should start with lower case in llvm.
Comment at: include-fixer/IncludeFixer.h:70
@@ +69,3 @@
+/// \return Replacements for inserting and sorting headers.
+std::vector CreateReplacementsForHeader(
+StringRef Code, StringRef FilePath, StringRef FallbackStyle,
Function name should start with lower case.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:211
@@ +210,3 @@
+
+ if (OuputHeaders) {
+// FIXME: Output IncludeFixerContext as YAML.
I'd put this before creating replacements and right after you get the context
since those replacements are not used here. And the "Added #include..." message
above doesn't make sense in this mode.
Comment at: include-fixer/tool/clang-include-fixer.py:49
@@ +48,3 @@
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
Maybe handle cases where `to_eval` is not a valid index?
Comment at: include-fixer/tool/clang-include-fixer.py:84
@@ +83,3 @@
+index = 1;
+for header in lines[1:]:
+ choices_message += "&" + str(index) + header + "\n"
If there is only one candidate, it doesn't make sense to ask users to choose it
IMO. We can simply insert the header in this case.
And I think we should make users pick the correct header if there are multiple
candidates by default. What do you think, Ben? @bkramer
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
bkramer added inline comments.
Comment at: include-fixer/IncludeFixer.h:51
@@ -50,6 +50,3 @@
- /// Headers to be added.
- std::set
-
- /// Replacements are written here.
- std::vector
+ /// The context that contains all information about queried symbol.
+ IncludeFixerContext
maybe 'about the symbol being queried'?
Comment at: include-fixer/IncludeFixer.h:72
@@ +71,3 @@
+StringRef Code, StringRef FilePath, StringRef FallbackStyle,
+unsigned FirstIncludeOffset, const std::string );
+
'Header' looks like it should be a StringRef too.
Comment at: include-fixer/IncludeFixerContext.h:19
@@ +18,3 @@
+
+struct IncludeFixerContext {
+ std::string SymbolIdentifer;
I think some doxygen comments would be in order for this struct and its members.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:62
@@ -58,1 +61,3 @@
+cl::opt OuputHeaders(
+"output-headers",
typo: Ouput
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
bkramer added a comment. Can you add some lit tests for the various command line modes clang-include-fixer has now. We can't reasonably test the vim integration but we can tests the bits it's composed of. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58448.
hokein marked 6 inline comments as done.
hokein added a comment.
Update and address comments.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::CreateReplacementsForHeader(
+Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -28,6 +28,34 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+choosing_mode = False
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
+ choosing_mode = vim.eval('g:clang_include_fixer_choosing_mode')
+
+
+def FillVimBuffer(stdout):
+ if stdout:
+lines = stdout.splitlines()
+sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
+for op in reversed(sequence.get_opcodes()):
+ if op[0] is not 'equal':
+vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message,
+ choices,
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,24 +69,42 @@
buf = vim.current.buffer
text = '\n'.join(buf)
+ if choosing_mode:
+# Run command to get all headers.
+command = [binary, "-output-headers", "-db="+args.db, "-input="+args.input,
+ vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+lines = stdout.splitlines()
+if len(lines) == 0:
+ return
+# The first line is the symbol name.
+symbol = lines[0]
+choices_message = ""
+index = 1;
+for header in lines[1:]:
+ choices_message += "&" + str(index) + header + "\n"
+ index += 1
+select = ShowDialog("choose a header file for {0}.".format(symbol),
+choices_message)
+# Insert a selected header.
+command = [binary, "-stdin", "-insert-header="+lines[select],
+ vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+FillVimBuffer(stdout)
+print "Added #include {0}.".format(lines[select])
+return;
+
+
# Call clang-include-fixer.
command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
# If successful, replace buffer contents.
if stderr:
print stderr
- if stdout:
-lines = stdout.splitlines()
-sequence = difflib.SequenceMatcher(None, vim.current.buffer, lines)
-for op in reversed(sequence.get_opcodes()):
- if op[0] is not 'equal':
-vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
+ FillVimBuffer(stdout)
if __name__ == '__main__':
main()
Index: include-fixer/tool/ClangIncludeFixer.cpp
===
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -9,13 +9,16 @@
#include "InMemorySymbolIndex.h"
#include "IncludeFixer.h"
+#include
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
ioeric added inline comments.
Comment at: include-fixer/IncludeFixer.cpp:241
@@ -280,5 +240,3 @@
/// \return true if changes will be made, false otherwise.
- bool Rewrite(clang::SourceManager ,
- clang::HeaderSearch ,
- std::set ,
- std::vector ) {
+ bool MinimizeAllIncludeHeaders(clang::SourceManager ,
+ clang::HeaderSearch ,
This doesn't seem to be the right name. It does more than minimizing headers.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:121
@@ +120,3 @@
+clang::include_fixer::CreateReplacementsForHeader(
+/*Code=*/Buffer.get()->getBuffer(),
+/*FilePath=*/FilePath,
These comments seem redundant since it is already clear what those parameters
are from variables' names.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:129
@@ +128,3 @@
+tooling::applyAllReplacements(Buffer.get()->getBuffer(), Replaces);
+llvm::outs() << ChangedCode;
+return 0;
This should only be for vim-mode or STDINMode I think. For normal mode, we
should apply replacements on the rewriter.
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:208
@@ +207,3 @@
+ clang::include_fixer::CreateReplacementsForHeader(
+ /*Code=*/Buffer.get()->getBuffer(),
+ /*FilePath=*/FilePath,
Same here.
Comment at: include-fixer/tool/clang-include-fixer.py:31
@@ -30,1 +30,3 @@
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
I think we can simply have one mode where a header is inserted if there is only
one result, and a dialog is shown only if there are multiple headers for user
to choose from.
Comment at: include-fixer/tool/clang-include-fixer.py:31
@@ -30,1 +30,3 @@
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
ioeric wrote:
> I think we can simply have one mode where a header is inserted if there is
> only one result, and a dialog is shown only if there are multiple headers for
> user to choose from.
Also remove semicolons here and below...
Comment at: include-fixer/tool/clang-include-fixer.py:78
@@ +77,3 @@
+stdout, stderr = execute(command, text)
+vim.current.buffer[:] = None;
+vim.current.buffer.append(stdout.splitlines())
I think we should also update the buffer with the diff method below. It doesn't
make sense to clear the buffer if we are just inserting one line.
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58420.
hokein added a comment.
Fix a nit.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::CreateReplacementsForHeader(
+Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -28,6 +28,22 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
+ choosing_mode = vim.eval('g:clang_include_fixer_choosing_mode')
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message, choices, default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,13 +57,34 @@
buf = vim.current.buffer
text = '\n'.join(buf)
+ if choosing_mode:
+# Run command to get all headers.
+command = [binary, "-output-headers", "-db="+args.db, "-input="+args.input,
+ vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+lines = stdout.splitlines()
+if len(lines) == 0:
+ return
+symbol = lines[0]
+choices_message = ""
+index = 1;
+for line in lines[1:]:
+ choices_message += "&" + str(index) + line + "\n"
+ index += 1
+select = ShowDialog("choose a header file for {0}.".format(symbol), choices_message)
+# Insert a selected header.
+command = [binary, "-insert-header="+lines[select], vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+vim.current.buffer[:] = None;
+vim.current.buffer.append(stdout.splitlines())
+print "Added #include {0}.".format(lines[select])
+return;
+
+
# Call clang-include-fixer.
command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
# If successful, replace buffer contents.
if stderr:
Index: include-fixer/tool/ClangIncludeFixer.cpp
===
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -9,13 +9,16 @@
#include "InMemorySymbolIndex.h"
#include "IncludeFixer.h"
+#include "IncludeFixerContext.h"
#include "SymbolIndexManager.h"
#include "YamlSymbolIndex.h"
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/ReplacementsYaml.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/YAMLTraits.h"
using namespace clang;
using namespace llvm;
@@ -56,6 +59,23 @@
"used for editor integration."),
cl::init(false), cl::cat(IncludeFixerCategory));
+cl::opt OuputHeaders(
+"output-headers",
+cl::desc("Output the queried symbol and its relevant headers.\n"
+ "The first line is the symbol name. The other lines\n"
+ "are the headers: \n"
+ " b::foo\n"
+ "
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein updated this revision to Diff 58415.
hokein added a comment.
Rebase
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
include-fixer/tool/clang-include-fixer.py
unittests/include-fixer/IncludeFixerTest.cpp
Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -70,11 +70,16 @@
SymbolIndexMgr->addSymbolIndex(
llvm::make_unique(Symbols));
- std::set Headers;
- std::vector Replacements;
- IncludeFixerActionFactory Factory(*SymbolIndexMgr, Headers, Replacements,
-"llvm");
+ IncludeFixerContext FixerContext;
+ IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
+
runOnCode(, Code, "input.cc", ExtraArgs);
+ std::vector Replacements;
+ if (!FixerContext.Headers.empty()) {
+Replacements = clang::include_fixer::CreateReplacementsForHeader(
+Code, "input.cc", "llvm", FixerContext.FirstIncludeOffset,
+FixerContext.Headers.front());
+ }
clang::RewriterTestContext Context;
clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -28,6 +28,22 @@
if vim.eval('exists("g:clang_include_fixer_path")') == "1":
binary = vim.eval('g:clang_include_fixer_path')
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
+ choosing_mode = vim.eval('g:clang_include_fixer_choosing_mode')
+
+def ShowDialog(message, choices, default_choice_index=0):
+ to_eval = "confirm('{0}', '{1}', '{2}')".format(message, choices, default_choice_index)
+ return int(vim.eval(to_eval));
+
+
+def execute(command, text):
+ p = subprocess.Popen(command,
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+ return p.communicate(input=text)
+
+
def main():
parser = argparse.ArgumentParser(
description='Vim integration for clang-include-fixer')
@@ -41,13 +57,34 @@
buf = vim.current.buffer
text = '\n'.join(buf)
+ if choosing_mode:
+# Run command to get all headers.
+command = [binary, "-output-headers", "-db="+args.db, "-input="+args.input,
+ vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+lines = stdout.splitlines()
+if len(lines) == 0:
+ return
+symbol = lines[0]
+choices_message = ""
+index = 1;
+for line in lines[1:]:
+ choices_message += "&" + str(index) + line + "\n"
+ index += 1
+select = ShowDialog("choose a header file for {0}.".format(symbol), choices_message)
+# Insert a selected header.
+command = [binary, "-insert-header="+lines[select], vim.current.buffer.name]
+stdout, stderr = execute(command, text)
+vim.current.buffer[:] = None;
+vim.current.buffer.append(stdout.splitlines())
+print "Added #include {0}.".format(lines[select])
+return;
+
+
# Call clang-include-fixer.
command = [binary, "-stdin", "-db="+args.db, "-input="+args.input,
vim.current.buffer.name]
- p = subprocess.Popen(command,
- stdout=subprocess.PIPE, stderr=subprocess.PIPE,
- stdin=subprocess.PIPE)
- stdout, stderr = p.communicate(input=text)
+ stdout, stderr = execute(command, text)
# If successful, replace buffer contents.
if stderr:
Index: include-fixer/tool/ClangIncludeFixer.cpp
===
--- include-fixer/tool/ClangIncludeFixer.cpp
+++ include-fixer/tool/ClangIncludeFixer.cpp
@@ -9,13 +9,16 @@
#include "InMemorySymbolIndex.h"
#include "IncludeFixer.h"
+#include "IncludeFixerContext.h"
#include "SymbolIndexManager.h"
#include "YamlSymbolIndex.h"
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/ReplacementsYaml.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/YAMLTraits.h"
using namespace clang;
using namespace llvm;
@@ -56,6 +59,23 @@
"used for editor integration."),
cl::init(false), cl::cat(IncludeFixerCategory));
+cl::opt OuputHeaders(
+"output-headers",
+cl::desc("Output the queried symbol and its relevant headers.\n"
+ "The first line is the symbol name. The other lines\n"
+ "are the headers: \n"
+ " b::foo\n"
+ "
Re: [PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.
hokein added a comment. Oh, sorry, I miss two separate commits here. This patch should not be ready for review. I need to rebase it after commit http://reviews.llvm.org/D20581. http://reviews.llvm.org/D20621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
