[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-15 Thread Zachary Turner via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289922: Add methods to enable using formatv syntax in LLDB. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D27632?vs=80992=81713#toc Repository: rL LLVM

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Sounds good. Take it as my vote this is ok to start with if we get consensus https://reviews.llvm.org/D27632 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. Thanks. Not going to commit this just yet, as I know there was at least one concern over on llvm-dev about adopting this. So I'll give it another few days to see if the complaints remain. https://reviews.llvm.org/D27632

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. So we need to formalize this in the formatv documentation before we begin wide adoption otherwise people can do what ever they want. It would be good to write this up so we get consistent

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Host/common/FileSpec.cpp:1394 + assert( + (Style.empty() || Style.equals_lower("F") || Style.equals_lower("D")) && + "Invalid FileSpec style!"); clayborg wrote: > zturner wrote: > > clayborg wrote: > >

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: source/Host/common/FileSpec.cpp:1394 + assert( + (Style.empty() || Style.equals_lower("F") || Style.equals_lower("D")) && + "Invalid FileSpec style!"); zturner wrote: > clayborg wrote: > > If we are going to

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Host/common/FileSpec.cpp:1394 + assert( + (Style.empty() || Style.equals_lower("F") || Style.equals_lower("D")) && + "Invalid FileSpec style!"); clayborg wrote: > If we are going to not care about case

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Looking good. A few little changes and possibly supporting long option names that can be minimally specified. See inlined comments. Comment at: source/Host/common/FileSpec.cpp:1394 + assert( + (Style.empty() || Style.equals_lower("F") ||

Re: [Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-12 Thread Pavel Labath via lldb-commits
On 10 December 2016 at 18:09, Zachary Turner wrote: > To elaborate about member function vs format provider, decoupling types from > the way they're formatted seems like a good design practice in general. For > example, you might want to add support for formatting a class

Re: [Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-10 Thread Zachary Turner via lldb-commits
To elaborate about member function vs format provider, decoupling types from the way they're formatted seems like a good design practice in general. For example, you might want to add support for formatting a class which you don't have the code to. Sure, you can do it one way whenever possible

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. This looks like a step in the right direction. I trust you won't go on a reformatting spree of the log messages until we settle on the syntax there. ;) Comment at: include/lldb/Core/ModuleSpec.h:244 + strm.Format("object_mod_time = {0:x+}",

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-09 Thread Zachary Turner via Phabricator via lldb-commits
zturner updated this revision to Diff 80992. zturner added a comment. Added a format provider for `FileSpec`. Style syntax is documented in this patch. Added some unit tests so you can see the output. To answer Greg's earlier question about printing c-strings, `formatv("'{0}' '{1}' '{2}'",

Re: [Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-09 Thread Zachary Turner via lldb-commits
Seems reasonable, I'll make a FileSpec formatter and update the patch On Fri, Dec 9, 2016 at 4:03 PM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg requested changes to this revision. > clayborg added a comment. > This revision now requires changes to proceed. > > I

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I would like to see the FileSpec having its formatv stuff done for this since it will start a whole slew of people wanting to use it and we should have an example of a class

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. It is nice to be able to use StringRef and std::string as is. I was wondering if you have: StringRef null; StringRef empty(""); StringRef hello("hello") What does this show: formatv("{0} {1} {2}", null, empty, hello); I would hope for: (null) "" "hello"

[Lldb-commits] [PATCH] D27632: Add Formatv() versions of all our printf style formatting functions

2016-12-09 Thread Zachary Turner via Phabricator via lldb-commits
zturner created this revision. zturner added reviewers: labath, clayborg, jingham. zturner added a subscriber: lldb-commits. We have various functions like `Stream::Printf()`, and `Error::SetErrorStringWithFormat()`, `Log::Printf()`, and various others. I added functions that delegate to