WIP: handle namespaced XML

[pfernie]

When putting together a test case for #72 , I created a .xlsx using Open XML SDK to be able to manually create both shared string and inline string data. It turns out that the xml generated by that will use namespaced XML tags (an example of such can be seen here, where, e.g., <x:sheets> is used instead of <sheets>. A sample .xlsx showing this behavior was added (richtext-namespaced.xlsx).

This PR introduces a trait LocalName which is used to strip namespace prefixes from tags for Bytes{Start,End}. However, such a method probably makes more sense added to quick-xml? (Or possibly this functionality already exists there and I missed it). So consider this PR a WIP; I'm happy to retool it to work w/ an updated quick-xml or another approach.

I also tried to make Event::Eof handling consistent by making it an error and explicitly checking for various closing tags.

[tafia]

I believe you could probably use read_namespaced_event, ignoring the resolved namespace?

[tafia]

Could you please also rebase?

[tafia]

Alternatively, if you don't want to use quick_xml namespaces resolution (which has some overhead), do you mind quickly benchmarking this alternative:

fn compare_name(node_name: &[u8], search_name: &[u8]) -> bool {
    node_name.ends_with(search_name)
        && (node_name.len() == search_name.len() 
            || node_name[node_name.len() - search_name.len() - 1] == b':') // as we are sure node_name.len() > search_name.len()
}

We could indeed probably amend quick-xml and provide this fn in BytesStart directly.

[pfernie]

I've rebased. Perhaps I'm misunderstanding the read_namespaced_event fn, but for a simple test case it doesn't seem to do what is needed (namely, strip the namespace prefix):

    let xml = r#"<?xml version="1.0" encoding="utf-8"?>
<x:workbook xmlns:x="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
    <x:sheets>
        <x:sheet name="mySheet" sheetId="1" r:id="Rddc7711f116045e5" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" />
    </x:sheets>
</x:workbook>"#;
    let mut rdr = Reader::from_str(xml);

    let mut buf = Vec::new();
    loop {
        match rdr.read_namespaced_event(&mut buf).unwrap() {
            (_, Event::Eof) => break,
            (_, Event::Start(ref e)) => println!("{}", ::std::str::from_utf8(e.name()).unwrap()),
            (_, Event::End(ref e)) => println!("{}", ::std::str::from_utf8(e.name()).unwrap()),
            _ => {}
        }
    }

still yields names with the namespace:

x:workbook
x:sheets
x:sheets
x:workbook

I did a simple (repeated runs w/ time) comparison of the existing local_name vs. the suggested compare_name that simply iterates over a worksheet with 416k cells, and there was no discernable difference (both run ~0.98s on my machine); they seem quite comparable.

The local_name approach feels slightly more ergonomic as it allows for e.g.

match e.local_name() {
  b"is" => {}
  b"v" => {}
  ...
}

But either way does seem like it should end up in quick-xml.

[tafia]

but for a simple test case it doesn't seem to do what is needed

Oh really! I'll check on quick-xml but it might be a regression then.

[tafia]

Just opened an issue on quick-xml.

I am fine merging this PR, I'll update quick-xml later. Except if you want to make that change on quick-xml directly, I cannot do it at the moment.

[pfernie]

After reviewing read_namespaced_event and realizing I had missed the extra step of resolve_namespace, I have submitted a PR tafia/quick-xml#60 which implements local_name there. I have an update of this branch that uses that, but obviously it will not build here until the quick-xml PR is resolved.

[tafia]

Should we close this PR and wait for merging the one on quick-xml first?

[pfernie]

I have a rebase of this PR which works with the quick-xml PR, so I can just update this PR if/once that is accepted.

[tafia]

Just published quick-xml 0.7.0

[pfernie]

PR updated to use quick-xml 0.7.0 and local_name. There seems to be some spurious failure on the AppVeyor build; it failed trying to run curl? Travis run went fine...

[tafia]

Sorry to bother you again, but I am not sure you need read_namespaced_event if you are dealing with namespaces manually via the local_name function anyway.

[tafia]

Can't you use read_event directly since the local_name is on BytesStart ?

[tafia]

There seems to be some spurious failure on the AppVeyor build; it failed trying to run curl?

We're not the only ones:
conda-forge/conda-forge.github.io#357

Looking at the case on AppVeyor could you try adding this to the appveyor file:

install:
- set PATH=C:\Program Files\Git\mingw64\bin;%PATH%

EDIT: I've just did it in #74, can you rebase?

[pfernie]

You're right; read_namespaced_event not needed with local_name; I switched them back to read_event and tidied up the branch. AppVeyor build is passing w/ your changes as well.

Thanks!

[tafia]

Thanks a lot for your patience!

[tafia]

Just published version 0.7.0