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

IDL support #232

Closed
wants to merge 4 commits into from
Closed

IDL support #232

wants to merge 4 commits into from

Conversation

halcyon-gh
Copy link
Contributor

@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

Merging #232 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
+ Coverage   99.23%   99.23%   +<.01%     
==========================================
  Files          65       65              
  Lines        6172     6176       +4     
==========================================
+ Hits         6125     6129       +4     
  Misses         47       47
Impacted Files Coverage Δ
jupytext/languages.py 100% <ø> (ø) ⬆️
tests/test_mirror.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a870b2...cbae18d. Read the comment docs.

@mwouts mwouts mentioned this pull request May 21, 2019
@mwouts
Copy link
Owner

mwouts commented May 21, 2019

Hello @halcyon-gh , thanks for your contribution. I will integrate this in the next release soon.

@mwouts
Copy link
Owner

mwouts commented May 21, 2019

This has been integrated into Jupytext 1.1.3. Thanks @halcyon-gh !

@mwouts mwouts closed this May 21, 2019
@mwouts
Copy link
Owner

mwouts commented Jun 14, 2019

Hello @halcyon-gh , I have a question for you... Yesterday I extended the round trip test for Jupyter notebooks as Markdown files to all languages, including IDL. But that did not work well as of 1.1.6, because the IDL language is sometimes encoded as IDL (in the kernel) or as idl (in our extension mapping). So we have to choose one... This is not a big issue, but I'd like to know your preference: should a code chunk, in Markdown, start with

```idl

or

```IDL

For now I have chosen the first one, see this file (go to the raw representation). Do you think it is OK?

@halcyon-gh
Copy link
Contributor Author

Hi, not really sure what is most appropriate. In the metadata of the official Exelis kernel you get this:

{
  "kernelspec": {
    "name": "idl",
    "display_name": "IDL",
    "language": "IDL"
  },
  "language_info": {
    "name": "idl",
    "codemirror_mode": "idl",
    "mimetype": "text/x-idl",
    "file_extension": ".pro"
  }
}

You would think kernelspec,language and language_info,name would be the same. Is one of those specifically queried by jupytext? Otherwise going with the lowercase version is as valid a choice as any.

@mwouts
Copy link
Owner

mwouts commented Jun 18, 2019

I see. Well, Jupytext takes the language from the kernelspec dictionary, so that's why until version 1.1.6 the Markdown representation had ```IDL. But I do think lower case is more appropriate. Since you also agree (thanks!) I'll go for ```idl in 1.1.7. I will also add an additional test to make sure that Markdown documents with the uppercase version can still be read.

mwouts added a commit that referenced this pull request Jun 20, 2019
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.

2 participants