Skip to content

Change #1

Open
fizzbucket wants to merge 8 commits into
elliottslaughter:masterfrom
fizzbucket:master
Open

Change #1
fizzbucket wants to merge 8 commits into
elliottslaughter:masterfrom
fizzbucket:master

Conversation

@fizzbucket

Copy link
Copy Markdown

This pull request updates the pandoc struct from a tuple struct to a c-type struct so that a hardcoded api version is no longer required. This will let the crate work with the latest versions of Pandoc. It also adds some more testcases and makes minor changes to 2018 edition idioms.

@elliottslaughter

Copy link
Copy Markdown
Owner

Hi there and thanks for putting this together.

I notice that there's also a fair amount of restructuring in this commit. Is that intentional, and if so is there any particular reason to favor the new structure?

@dkasak

dkasak commented May 23, 2020

Copy link
Copy Markdown

I'm curious about this as well as I need support for the latest pandoc-types. Pinging @fizzbucket.

@elliottslaughter, are you primarily referring to the removal of the definition module?

@elliottslaughter

Copy link
Copy Markdown
Owner

Right, there seem to be four distinct kinds of changes in this commit:

  1. Modernization, e.g. updating to 2018 editition and serde usage.
  2. Updating the definitions to be compatible with newer Pandoc types versions.
  3. Moving all definitions into the root of the crate.
  4. Adding test cases.

I'm happy with all of these except (3), which just seems like an anti-pattern to me. Maybe I'm wrong though. Or maybe we could use definition::* inside of lib.rs so we're not literally moving all the definitions. At any rate, if we're going to consider this piece, I'd want to at least have a discussion about it first, and talk about the tradeoffs.

I don't have time at the moment, but maybe in the next couple of evenings I can try to find time to split out (1), (2) and (4) so they can be committed into the repo separately, which would at least unblock users who want to use newer Pandoc Types versions. And then we can have the API discussion separately.

@elliottslaughter

Copy link
Copy Markdown
Owner

I was looking at this and realized that the examples directory did not get updated. As far as I can tell, there are no examples of directly creating Pandoc objects in the tests. So that's a use case that isn't being exercised in this PR. It also wasn't clear to me how a user was supposed to get the api_version when creating the Pandoc object.

For now I've just bumped the API version to 1.20. This is sufficient to make my local tests pass.

I would be open to further improvements to require less hard-coding, but I'd like to know how we're going to solve the issue with users needing to know what API version to create documents for.

@dkasak if you pull you should be able to use this with the latest pandoc-types.

@elliottslaughter

Copy link
Copy Markdown
Owner

Ok, I think I've pulled in all of (1), (2) and (4), and have made a new release on crates.io. Please update your Cargo.toml to specify 0.3 and see if it works for you now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants