On Feb 20, 2014, at 4:34 PM, Ben Langmuir <[email protected]> wrote:

> 
> On Feb 20, 2014, at 4:11 PM, Ben Langmuir <[email protected]> wrote:
> 
>> 
>> On Feb 20, 2014, at 11:22 AM, Argyrios Kyrtzidis <[email protected]> wrote:
>> 
>>> On Feb 20, 2014, at 10:02 AM, Ben Langmuir <[email protected]> wrote:
>>> 
>>>> 
>>>>> I think it is better to move the YAML VFS description to a new file under 
>>>>> 'docs/'. This file format is API, and should be documented.
>>>> 
>>>> Can this wait until we have more practical experience with the file 
>>>> format? Things in docs/ tend to have better documentation that what I've 
>>>> written here, and I don't want to put a lot of effort into this until 
>>>> we're sure this is the right format.
>>> 
>>> +1, we are far from the point of committing to API, this is still in flux.
>>> 
>>>> 
>>>>> Unrelated to this patch, but how do we keep track of the current working 
>>>>> directory? Or we don't just yet?
>>>> 
>>>> We don't :)  I'll look at whether that is simple to do now, or can wait.  
>>>> It won't be exercised until we use the VFS outside of FileManager, since 
>>>> FileManager always creates absolute paths I think.
>>> 
>>> FileManager has FileSystemOptions which has the “-working-directory” 
>>> option, we should probably take advantage of that.
>>> 
>>>> 
>>>> ================
>>>> Comment at: lib/Basic/VirtualFileSystem.cpp:260
>>>> @@ +259,3 @@
>>>> +/// Possible configuration settings are
>>>> +///   'case-sensitive': <boolean>
>>>> +///
>>>> ----------------
>>>> Dmitri Gribenko wrote:
>>>>> Is it optional?  If so, what is the default?
>>>>> 
>>>>> Please also add a mandatory version field, and refuse to load files with 
>>>>> incorrect version.  I hope we will never have to bump the version and 
>>>>> evolve the format in a compatible way, but who knows...
>>>> It is optional, which is mentioned above.  I will specify its default 
>>>> value.
>>>> 
>>>> The version is going to make my unit tests ugly, but I see your point.
>>> 
>>> Do we need something standard at the beginning (something equivalent to a 
>>> "<?xml version="1.0" encoding="UTF-8”?>”) that we can sniff out to figure 
>>> out if it is json or binary format ?
>>> I would prefer that if we move to binary format, the build system would 
>>> need no changes at all (e.g. not require that the build system changes the 
>>> file extension).
>>> 
>> 
>> We could use the YAML version string
>> %YAML 1.2
> 
> Actually, if we go this route we should encode our own version number in the 
> same place for compactness.  In YAML, we could use a tag for this
> 
> %TAG ! tag:vfs-yaml-file,version:1234
> 
> Where 1234 is a made up version number for our vfs format.

SGTM.

> 
> Ben
> 
>> 
>> It’s not legal JSON, but I don’t see any reason to be strict about JSON 
>> conformance as long as our parser works.
>> 
>> Ben

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to