Re: [clang-tools-extra] r325097 - [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

[Ilya Biryukov via cfe-commits]

On Thu, Feb 15, 2018 at 8:29 AM Sam McCall  wrote:

> On Wed, Feb 14, 2018 at 10:45 AM Ilya Biryukov 
> wrote:
>
>> Personally, I'm not a big fan of environment variables. There are harder
>> to discover than command-line flags and I still have to change editor
>> config often to switch between different builds of clangd.
>> I know it may sound weird, but maybe we could have both the flag and the
>> environment variables? (flag taking the precedence if both are specified)
>>
> Do you use tracing often? Can you describe your workflow?
>
Reasonably often if I need to use it.
It's also easier to change config flag in vscode and hit "debug" than to
set the global environment variable (I happen to often run clangd from
debug instance of VSCode, not from the command line).

(Less concerned about discoverability - this was always a hidden flag
> anyway)
>
A hidden flag is still more discoverable than environment variable.


>
>
>> On Wed, Feb 14, 2018 at 4:22 AM Sam McCall via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sammccall
>>> Date: Tue Feb 13 19:20:07 2018
>>> New Revision: 325097
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=325097=rev
>>> Log:
>>> [clangd] Configure clangd tracing with CLANGD_TRACE env instead of
>>> -trace flag
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>>> clang-tools-extra/trunk/test/clangd/trace.test
>>>
>>> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=325097=325096=325097=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
>>> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Feb 13
>>> 19:20:07 2018
>>> @@ -17,6 +17,7 @@
>>>  #include "llvm/Support/Path.h"
>>>  #include "llvm/Support/Program.h"
>>>  #include "llvm/Support/raw_ostream.h"
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -123,12 +124,6 @@ static llvm::cl::opt InputMirrorFi
>>>  "Mirror all LSP input to the specified file. Useful for
>>> debugging."),
>>>  llvm::cl::init(""), llvm::cl::Hidden);
>>>
>>> -static llvm::cl::opt TraceFile(
>>> -"trace",
>>> -llvm::cl::desc(
>>> -"Trace internal events and timestamps in chrome://tracing JSON
>>> format"),
>>> -llvm::cl::init(""), llvm::cl::Hidden);
>>> -
>>>  static llvm::cl::opt EnableIndexBasedCompletion(
>>>  "enable-index-based-completion",
>>>  llvm::cl::desc(
>>> @@ -176,15 +171,18 @@ int main(int argc, char *argv[]) {
>>>  }
>>>}
>>>
>>> -  // Setup tracing facilities.
>>> +  // Setup tracing facilities if CLANGD_TRACE is set. In practice
>>> enabling a
>>> +  // trace flag in your editor's config is annoying, launching with
>>> +  // `CLANGD_TRACE=trace.json vim` is easier.
>>>llvm::Optional TraceStream;
>>>std::unique_ptr Tracer;
>>> -  if (!TraceFile.empty()) {
>>> +  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
>>>  std::error_code EC;
>>>  TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
>>>  if (EC) {
>>> -  TraceFile.reset();
>>> -  llvm::errs() << "Error while opening trace file: " <<
>>> EC.message();
>>> +  TraceStream.reset();
>>> +  llvm::errs() << "Error while opening trace file " << TraceFile <<
>>> ": "
>>> +   << EC.message();
>>>  } else {
>>>Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
>>>  }
>>>
>>> Modified: clang-tools-extra/trunk/test/clangd/trace.test
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=325097=325096=325097=diff
>>>
>>> ==
>>> --- clang-tools-extra/trunk/test/clangd/trace.test (original)
>>> +++ clang-tools-extra/trunk/test/clangd/trace.test Tue Feb 13 19:20:07
>>> 2018
>>> @@ -1,4 +1,4 @@
>>> -# RUN: clangd -lit-test -trace %t < %s && FileCheck %s < %t
>>> +# RUN: CLANGD_TRACE=%t clangd -lit-test < %s && FileCheck %s < %t
>>>
>>>  
>>> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
>>>  ---
>>>  
>>> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
>>> main() {}"}}}
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>

-- 
Regards,
Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r325097 - [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

[Sam McCall via cfe-commits]

On Wed, Feb 14, 2018 at 10:45 AM Ilya Biryukov  wrote:

> Personally, I'm not a big fan of environment variables. There are harder
> to discover than command-line flags and I still have to change editor
> config often to switch between different builds of clangd.
> I know it may sound weird, but maybe we could have both the flag and the
> environment variables? (flag taking the precedence if both are specified)
>
Do you use tracing often? Can you describe your workflow?

(Less concerned about discoverability - this was always a hidden flag
anyway)


> On Wed, Feb 14, 2018 at 4:22 AM Sam McCall via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: sammccall
>> Date: Tue Feb 13 19:20:07 2018
>> New Revision: 325097
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=325097=rev
>> Log:
>> [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace
>> flag
>>
>> Modified:
>> clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>> clang-tools-extra/trunk/test/clangd/trace.test
>>
>> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=325097=325096=325097=diff
>>
>> ==
>> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
>> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Feb 13
>> 19:20:07 2018
>> @@ -17,6 +17,7 @@
>>  #include "llvm/Support/Path.h"
>>  #include "llvm/Support/Program.h"
>>  #include "llvm/Support/raw_ostream.h"
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -123,12 +124,6 @@ static llvm::cl::opt InputMirrorFi
>>  "Mirror all LSP input to the specified file. Useful for
>> debugging."),
>>  llvm::cl::init(""), llvm::cl::Hidden);
>>
>> -static llvm::cl::opt TraceFile(
>> -"trace",
>> -llvm::cl::desc(
>> -"Trace internal events and timestamps in chrome://tracing JSON
>> format"),
>> -llvm::cl::init(""), llvm::cl::Hidden);
>> -
>>  static llvm::cl::opt EnableIndexBasedCompletion(
>>  "enable-index-based-completion",
>>  llvm::cl::desc(
>> @@ -176,15 +171,18 @@ int main(int argc, char *argv[]) {
>>  }
>>}
>>
>> -  // Setup tracing facilities.
>> +  // Setup tracing facilities if CLANGD_TRACE is set. In practice
>> enabling a
>> +  // trace flag in your editor's config is annoying, launching with
>> +  // `CLANGD_TRACE=trace.json vim` is easier.
>>llvm::Optional TraceStream;
>>std::unique_ptr Tracer;
>> -  if (!TraceFile.empty()) {
>> +  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
>>  std::error_code EC;
>>  TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
>>  if (EC) {
>> -  TraceFile.reset();
>> -  llvm::errs() << "Error while opening trace file: " << EC.message();
>> +  TraceStream.reset();
>> +  llvm::errs() << "Error while opening trace file " << TraceFile <<
>> ": "
>> +   << EC.message();
>>  } else {
>>Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
>>  }
>>
>> Modified: clang-tools-extra/trunk/test/clangd/trace.test
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=325097=325096=325097=diff
>>
>> ==
>> --- clang-tools-extra/trunk/test/clangd/trace.test (original)
>> +++ clang-tools-extra/trunk/test/clangd/trace.test Tue Feb 13 19:20:07
>> 2018
>> @@ -1,4 +1,4 @@
>> -# RUN: clangd -lit-test -trace %t < %s && FileCheck %s < %t
>> +# RUN: CLANGD_TRACE=%t clangd -lit-test < %s && FileCheck %s < %t
>>
>>  
>> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
>>  ---
>>  
>> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
>> main() {}"}}}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r325097 - [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

[Ilya Biryukov via cfe-commits]

Personally, I'm not a big fan of environment variables. There are harder to
discover than command-line flags and I still have to change editor config
often to switch between different builds of clangd.
I know it may sound weird, but maybe we could have both the flag and the
environment variables? (flag taking the precedence if both are specified)


On Wed, Feb 14, 2018 at 4:22 AM Sam McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sammccall
> Date: Tue Feb 13 19:20:07 2018
> New Revision: 325097
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325097=rev
> Log:
> [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace
> flag
>
> Modified:
> clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> clang-tools-extra/trunk/test/clangd/trace.test
>
> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=325097=325096=325097=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Feb 13 19:20:07
> 2018
> @@ -17,6 +17,7 @@
>  #include "llvm/Support/Path.h"
>  #include "llvm/Support/Program.h"
>  #include "llvm/Support/raw_ostream.h"
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -123,12 +124,6 @@ static llvm::cl::opt InputMirrorFi
>  "Mirror all LSP input to the specified file. Useful for
> debugging."),
>  llvm::cl::init(""), llvm::cl::Hidden);
>
> -static llvm::cl::opt TraceFile(
> -"trace",
> -llvm::cl::desc(
> -"Trace internal events and timestamps in chrome://tracing JSON
> format"),
> -llvm::cl::init(""), llvm::cl::Hidden);
> -
>  static llvm::cl::opt EnableIndexBasedCompletion(
>  "enable-index-based-completion",
>  llvm::cl::desc(
> @@ -176,15 +171,18 @@ int main(int argc, char *argv[]) {
>  }
>}
>
> -  // Setup tracing facilities.
> +  // Setup tracing facilities if CLANGD_TRACE is set. In practice
> enabling a
> +  // trace flag in your editor's config is annoying, launching with
> +  // `CLANGD_TRACE=trace.json vim` is easier.
>llvm::Optional TraceStream;
>std::unique_ptr Tracer;
> -  if (!TraceFile.empty()) {
> +  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
>  std::error_code EC;
>  TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
>  if (EC) {
> -  TraceFile.reset();
> -  llvm::errs() << "Error while opening trace file: " << EC.message();
> +  TraceStream.reset();
> +  llvm::errs() << "Error while opening trace file " << TraceFile <<
> ": "
> +   << EC.message();
>  } else {
>Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
>  }
>
> Modified: clang-tools-extra/trunk/test/clangd/trace.test
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=325097=325096=325097=diff
>
> ==
> --- clang-tools-extra/trunk/test/clangd/trace.test (original)
> +++ clang-tools-extra/trunk/test/clangd/trace.test Tue Feb 13 19:20:07 2018
> @@ -1,4 +1,4 @@
> -# RUN: clangd -lit-test -trace %t < %s && FileCheck %s < %t
> +# RUN: CLANGD_TRACE=%t clangd -lit-test < %s && FileCheck %s < %t
>
>  
> {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
>  ---
>  
> {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
> main() {}"}}}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


-- 
Regards,
Ilya Biryukov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r325097 - [clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Feb 13 19:20:07 2018
New Revision: 325097

URL: http://llvm.org/viewvc/llvm-project?rev=325097=rev
Log:
[clangd] Configure clangd tracing with CLANGD_TRACE env instead of -trace flag

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/trace.test

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=325097=325096=325097=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Feb 13 19:20:07 2018
@@ -17,6 +17,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -123,12 +124,6 @@ static llvm::cl::opt InputMirrorFi
 "Mirror all LSP input to the specified file. Useful for debugging."),
 llvm::cl::init(""), llvm::cl::Hidden);
 
-static llvm::cl::opt TraceFile(
-"trace",
-llvm::cl::desc(
-"Trace internal events and timestamps in chrome://tracing JSON 
format"),
-llvm::cl::init(""), llvm::cl::Hidden);
-
 static llvm::cl::opt EnableIndexBasedCompletion(
 "enable-index-based-completion",
 llvm::cl::desc(
@@ -176,15 +171,18 @@ int main(int argc, char *argv[]) {
 }
   }
 
-  // Setup tracing facilities.
+  // Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a
+  // trace flag in your editor's config is annoying, launching with
+  // `CLANGD_TRACE=trace.json vim` is easier.
   llvm::Optional TraceStream;
   std::unique_ptr Tracer;
-  if (!TraceFile.empty()) {
+  if (auto *TraceFile = getenv("CLANGD_TRACE")) {
 std::error_code EC;
 TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
 if (EC) {
-  TraceFile.reset();
-  llvm::errs() << "Error while opening trace file: " << EC.message();
+  TraceStream.reset();
+  llvm::errs() << "Error while opening trace file " << TraceFile << ": "
+   << EC.message();
 } else {
   Tracer = trace::createJSONTracer(*TraceStream, PrettyPrint);
 }

Modified: clang-tools-extra/trunk/test/clangd/trace.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/trace.test?rev=325097=325096=325097=diff
==
--- clang-tools-extra/trunk/test/clangd/trace.test (original)
+++ clang-tools-extra/trunk/test/clangd/trace.test Tue Feb 13 19:20:07 2018
@@ -1,4 +1,4 @@
-# RUN: clangd -lit-test -trace %t < %s && FileCheck %s < %t
+# RUN: CLANGD_TRACE=%t clangd -lit-test < %s && FileCheck %s < %t
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void
 main() {}"}}}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits