ilya-biryukov added inline comments.

================
Comment at: clangd/Logger.cpp:19
 Logger *L = nullptr;
+bool Verbose_ = false;
 } // namespace
----------------
simark wrote:
> ilya-biryukov wrote:
> > Could we move the flag to implementation of `Logger`?
> > I.e.:
> > ```
> > class Logger {
> >   virtual log(const llvm::Twine &Message, bool Verbose);
> > };
> > 
> > // Implementation of top-level log
> > void clangd::log(const llvm::Twine &Message) {
> >    L->log(Message, /*Verbose=*/false);
> >    // should also handle missing Logger by logging into llvm::errs()
> > }
> > 
> > // Implementation of top-level vlog.
> > void clangd::vlog(const llvm::Twine &Message) {
> >    L->log(Message, /*Verbose=*/true);
> >    // should also handle missing Logger by logging into llvm::errs()
> > }
> > ```
> > 
> > An implementation of the interface would decide whether to log or not, 
> > based on command-line argument.
> That's what I thought of doing first.  The issue is that if we don't set a 
> logger with LoggingSession, the log function prints to stderr itself.  So if 
> we don't check the Verbose flag there, the -v flag will have no effect when 
> LoggingSession has not been called (I don't know if it's really a problem or 
> not, since in practice we call it).
It shouldn't be a problem.
We're only missing `LoggingSession` in tests, so let's just print messages from 
`vlog` into `stderr` in the same way we do it with `log` now.


================
Comment at: clangd/tool/ClangdMain.cpp:79
 
+static llvm::cl::opt<bool> Verbose("verbose", llvm::cl::desc("Be more 
verbose"),
+                                   llvm::cl::init(false));
----------------
simark wrote:
> ilya-biryukov wrote:
> > Maybe just call it `-v`?
> I would have like to add both "-v" and "-verbose", but it doesn't seem 
> possible to have two flags for the same option.  "-v" it is then, it is quite 
> standard.
I would go with having just `-v` with no aliases.

But this should do the trick if you prefer to have `-verbose` as an option:
```
llvm::cl::opt<bool> Verbose("v", llvm::cl::alias("verbose") , //....
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226



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

Reply via email to