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

Support multiple language mappings? #684

Closed
maximsch2 opened this issue Apr 1, 2017 · 7 comments
Closed

Support multiple language mappings? #684

maximsch2 opened this issue Apr 1, 2017 · 7 comments
Labels
bug 🐛 For unexpected issues

Comments

@maximsch2
Copy link
Contributor

I'm using Hydrogen to execute code in Julia files and also wanted to try out Weave.jl. It has it's own Atom language file that suggests using language mappings to get Hydrogen to execute the code: https://github.com/mpastell/language-weave

The issue is, if I set up custom mapping to enable Julia code execution in "weave.jl markdown" files, then I lose that ability in regular "julia" files. It would be great if language mappings would allow to either specify a list of types for the mapping or just the mapping was reversed.

@n-riesco
Copy link
Collaborator

n-riesco commented Apr 1, 2017

I think the language mappings suggested in https://github.com/mpastell/language-weave are meant for a very old version of Hydrogen.

If I understand correctly, the grammar name is "weave.jl markdown" and the kernel name is "julia". If so, the correct language mapppings are these:

{ "weave.jl markdown": "julia", "pweave markdown": "python" }

If this doesn't work, please, could you post the error message?

@lgeiger
Copy link
Member

lgeiger commented Apr 1, 2017

@n-riesco I think the language mappings from weave are correct.
The problem is that if a language mapping is present, Hydrogen doesn't recognize the original language. For example it only works with weave.jl markdow files and not with julia files anymore.

For the specific case of weave, instead of using a language mapping the best way would be if they could integrate with our new multi language feature (which should be very easy).

@n-riesco
Copy link
Collaborator

n-riesco commented Apr 1, 2017

I think the language mappings from weave are correct.

Oh! I keep confusing that the languageMappings setting maps kernel language names into atom grammar names.

@lgeiger I think a solution to this issue would be to replace these lines with

    if (mappedLanguage) {
      return mappedLanguage === language || kernelLanguage.toLowerCase() === language;
    }

In this way, Hydrogen would still consider the julia kernel for julia files despite the languageMappings setting.

@maximsch2 Alternatively, you could create a custom kernel spec by adding something like this to the KernelSpec setting:

      {
          "kernelspecs": {
              "weave.jl markdown": {
                  "spec": {
                      "argv": [
                          "julia",
                          "-i",
                          "-F",
                          "/home/user/.julia/v0.5/IJulia/src/kernel.jl",
                          "{connection_file}"
                      ],
                      "display_name": "Weave.jl markdown",
                      "language": "weave.jl markdown"
                  }
              }
          }
      }

@lgeiger
Copy link
Member

lgeiger commented Apr 1, 2017

I think a solution to this issue would be to replace these lines with

That's probably part of the solution. I think we also need to change our grammarToLanguage function, since the running kernels are stored by the atom grammar name and not by their language from the kernelspec.

Alternatively, you could create a custom kernel spec by adding something like this to the KernelSpec setting

That's a valid workaround, though I wouldn't recommend editing the kernelspecs setting as a long term solution since this will likely fail if the julia kernel is updated to a new version.

@n-riesco
Copy link
Collaborator

n-riesco commented Apr 1, 2017

That's probably part of the solution. I think we also need to change our grammarToLanguage function, since the running kernels are stored by the atom grammar name and not by their language from the kernelspec.

Yes, it makes a lot of sense.

@lgeiger lgeiger added the bug 🐛 For unexpected issues label Apr 1, 2017
@lgeiger
Copy link
Member

lgeiger commented Apr 1, 2017

@maximsch2 Would you be interested in implementing multi language support for weave as a first class citizen in Hydrogen?

As an example here's what we did to support the knitr language:
christophergandrud/language-knitr#4
#663

@maximsch2
Copy link
Contributor Author

@lgeiger I have a version that seems to be working, let me make a PR.

Thanks for a pointer!

lgeiger added a commit to lgeiger/hydrogen that referenced this issue Apr 1, 2017
This will store running kernels by the language from their kernel spec.
It also makes sure that all names are normalized to lower case to avoid confusion.

Fixes nteract#684
rgbkrk pushed a commit that referenced this issue Apr 2, 2017
This will store running kernels by the language from their kernel spec.
It also makes sure that all names are normalized to lower case to avoid confusion.

Fixes #684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 For unexpected issues
Projects
None yet
Development

No branches or pull requests

3 participants