Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for zimwiki writer #3403

Closed
wants to merge 92 commits into from
Closed

Fixes for zimwiki writer #3403

wants to merge 92 commits into from

Conversation

alexivkin
Copy link
Contributor

Improvements for ZimWiki's source view and table markup.


-- | Convert Pandoc to ZimWiki.
writeZimWiki :: PandocMonad m => WriterOptions -> Pandoc -> m String
writeZimWiki opts document = return $ evalState (pandocToZimWiki opts document) (WriterState 1 "")
writeZimWiki opts document = return $ evalState (pandocToZimWiki opts document) (WriterState 1 "" False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WriterState seems to be def, maybe it would make sense to use that instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Def?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The def value from the Default type class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea how to do that. Are there any existing examples in the code?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what @tarleb is proposing is simply: evalState (pandocToZimWiki opts document) def

@alexivkin
Copy link
Contributor Author

alexivkin commented Feb 14, 2017

Fixed to def and added 2 more fixes. Not sure why travis checks never complete.

@alexivkin alexivkin changed the title Fixes for sourceview and table plugins Fixes for zimwiki writer Feb 14, 2017
@mb21
Copy link
Collaborator

mb21 commented Feb 14, 2017

@alexivkin e.g. this travis job is failing because of the zimwiki test. Try stack test to run the tests on your local system... and update the /test/writer.zimwiki file if necessary...

@alexivkin
Copy link
Contributor Author

updated the tests

jgm and others added 22 commits February 14, 2017 11:11
Add a -MacOS suffix to mac package rather than -OSX.
CHanged local names from osx to macos.
This comment is likely copied from RST.hs where 'refs' variable indeed exists, but makes no sense here.
This just parses inside smartTags and yields their contents,
ignoring the attributes of the smartTag.  @jkr, you may want
to adjust this, but I wanted to get a fix in as fast as possible
for the dropped content.

Closes #2242; see also #3412.
Previously we didn't recognize math, for example, when
the xmlns declaration occured on the element and not the root.
Now we recognize either.

Closes #3365.

This patch defines findChildByName, findChildrenByName,
and findAttrByName in Util, and uses these in Parse.
We should make this closer to the actual options,
and do processing outside.
Moved unsmartify to Writers.Shared.
jgm and others added 24 commits February 14, 2017 11:11
Verbosity level only affects which are printed to stdout.
(Exception: DEBUG messages are only printed, never saved to
state.)
Use a deterministic order for fields.
See #3432.  Previously the parser didn't handle properly this
case:

    * - a
      - b
    * - c
      - d
This allows adding captions to tables.
Currently the support for the `.. table` directive is a bit
limited; we don't yet support the `widths` field.  But at least
you can have a proper captioned table.
This is deprecated but may still be in older documents.
I don't think this is still needed. If the Travis build fails,
we can try adjusting the CPP to make it more compatible.
For example, in

     \begin{tabular}{>{$}l<{$}>{$}l<{$} >{$}l<{$}}

each cell will be interpreted as if it has a `$`
before its content and a `$` after (math mode).
@alexivkin
Copy link
Contributor Author

The tests work for zim, but fail on Latex - completely unrelated to my code. It's something in the previous main branch commits that triggered it.

@jgm
Copy link
Owner

jgm commented Feb 14, 2017

The rebase has gotten pretty hairy. You added this at a bad time, with lots of churn in the code base. Sorry this has been difficult.

Suggestion:

  1. remove this PR
  2. copy your src/Text/Pandoc/Writers/ZimWiki.hs, test/writer.zimwiki, and test/table.zimwiki to a safe place.
  3. check out current pandoc master
  4. copy the three files backed up above to their appropriate places in master
  5. git add and commit them. Be sure to include a full commit message, describing the changes you've made..."fixed the writer" is not enough. I'll need this for the changelog.
  6. stack test
  7. assuming all goes well, create a new PR. This will be a single clean commit on top of master, with a good description of the changes, and should be mergeable.

@alexivkin alexivkin closed this Feb 15, 2017
@alexivkin
Copy link
Contributor Author

Done. #3446
Should have I committed to a branch, instead of master, to avoid trying to catch up with the head?

@mb21
Copy link
Collaborator

mb21 commented Feb 15, 2017

great!

Should have I committed to a branch, instead of master, to avoid trying to catch up with the head?

commiting to a branch frees the master branch of your own repo to work on other features while you wait for jgm to merge your pull request into the official master. you always have to catch up with master (best to rebase, not merge), but this time you were just unlucky that there were a lot of changes in the meantime.

@jgm
Copy link
Owner

jgm commented Feb 15, 2017 via email

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.

6 participants