Js Port #83
Js Port #83
Conversation
Co-authored-by: Guillaume Grossetie <[email protected]>
…ex into feature/jsPort
Co-authored-by: Guillaume Grossetie <[email protected]>
| "ProgramFan (https://github.com/ProgramFan)", | ||
| "Leon De Andrade (https://github.com/Leon0402)" | ||
| ], | ||
| "license": "Open Works", |
There was a problem hiding this comment.
Not sure about this licence.
@mojavelinux do we have guidelines in the Asciidoctor org?
There was a problem hiding this comment.
I got this from the repo: https://github.com/asciidoctor/asciidoctor-bibtex/blob/master/LICENSE.txt
There was a problem hiding this comment.
Yes but I'm wondering if we should replace this license to align with other projects in the Asciidoctor organization.
There was a problem hiding this comment.
The Asciidoctor project doesn't use the Open Works license. It should either be MIT or Apache. I prefer MIT. Open Works is compatible with MIT as far as I can tell (since it is derived from it and extended to non-software) so I don't think there is any issue with updating it.
Co-authored-by: Guillaume Grossetie <[email protected]>
| result << "[[#{key}]]" if @links | ||
| if StyleUtils.is_numeric? @style | ||
| result << "#{@bibtex_ob}#{index}#{@bibtex_cb} " | ||
| if RUBY_ENGINE == 'opal' and not cptext.nil? | ||
| # Numbering is added by asciidoctor-bibtex and therefore should be stripped here | ||
| cptext = cptext.first.gsub(/^[\[\d\.\]]*/, ""), "" |
There was a problem hiding this comment.
You meant by Citation.js? Otherwise, I don't get why we do it only when RUBY_ENGINE == 'opal'
There was a problem hiding this comment.
I meant asciidoctor-bibtex already adds numbering in the bibliography. But so does citation.js. So both add numbers.
Actually the numbers from citation.js depend on the style. For instance for vancouver it is "1. MyBibEntry" whereas for ieee it is "[1] MyBibEntry". For asciidoctor-bibtex it is alwas [1].
To be consistent with asciidoctor-bibtex I strip the numbering from citation.js
There was a problem hiding this comment.
Understood.
I would say that "Strip Citation.js numbering, will be added by asciidoctor-bibtex"?
I think it's easier to understand if we explain what we are doing and then why. When I first read the sentence, I thought the goal was to remove the numbering added by asciidoctor-bibtex (and use Citation.js numbering but apparently that's the other way around).
|
I think it would be great to add a smoke test to make sure that the generation is working as expected. Something like: https://github.com/asciidoctor/asciidoctor-reveal.js/blob/acb2247380fe70e51dadcbb71568e2902d8865ce/test/smoke.js#L1-L22 |
| @locale = options[:locale] | ||
|
|
||
| styleFilePath = "../vendor/styles/#{@style}.csl" | ||
| styleFilePath = `path.resolve(__dirname, #{styleFilePath})` |
There was a problem hiding this comment.
@Mogztter I needed to add this, so it works regardless of the structure. I'm sure this can be written in ruby, but I'm not quite sure how. I tried a few things like styleFilePath == File.expand_path(styleFilePath, __FILE__).
There was a problem hiding this comment.
What File.expand_path(styleFilePath, __FILE__) gives you? Maybe __FILE__ is not correctly implemented in Opal... 🤔
There was a problem hiding this comment.
I think it might be a bug in opal. I tried following code snippet:
puts __FILE__
puts File.expand_path('../vendor/styles', __FILE__)
With ruby __FILE__ just gives the file name and expand_path the full absolute path.
Wit opal (our version & newest) you get the same behaviour for __FILE__, but expand_path just gives ./vendor/styles.
So it seems expand_path or __FILE__ is not correctly implemented.
There was a problem hiding this comment.
It would be better to use File.absolute_path. File.expand_path has some magic behavior that isn't needed here.
Fixes #48
This is my initial try to port asciidoctor-bibtex to javascript. I tested it with one of the more complex examples with different styles and it seems to work fine.