riboseinc / asciidoctor-bibliography

Citations in AsciiDoc
MIT License
30 stars 8 forks source link

version 0.8.0 didn't work for me #79

Open andrewcarver opened 6 years ago

andrewcarver commented 6 years ago

Folks, yesterday I updated from 0.7.2 to 0.8.0. It broke it: gave me an error that the first thng I cited was not in the database file (which it was). I uninstalled 0.8.0 (which put me back to 0.7.2), and everything worked.

I know I'm using a slightly older version of Ruby: ruby 2.4.2p198 (2017-09-14 revision 59899) [x64-mingw32] Could that be the issue? Thanks.

P.S. The citation about which it complained was in a comment block.

andrewcarver commented 6 years ago

The problem persisted following an upgrade of Ruby. Here is the trace I'm getting:

Traceback (most recent call last): 28: from C:/tools/ruby25/bin/asciidoctor:23:in '

' 27: from C:/tools/ruby25/bin/asciidoctor:23:in 'load' 26: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/bin/asciidoctor:14:in '<top (required)>' 25: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/cli/invoker.rb:112:in 'invoke!' 24: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/cli/invoker.rb:112:in 'each' 23: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/cli/invoker.rb:129:in 'block in invoke!' 22: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor.rb:1589:in 'convert_file' 21: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor.rb:1589:in 'open' 20: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor.rb:1589:in 'block in convert_file' 19: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor.rb:1473:in 'convert' 18: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor.rb:1361:in 'load' 17: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/document.rb:559:in 'parse' 16: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/document.rb:559:in 'each' 15: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/document.rb:560:in 'block in parse' 14: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-1.5.7/lib/asciidoctor/document.rb:560:in '[]' 13: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:18:in 'process' 12: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:29:in 'process_lines' 11: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:44:in 'render_citations' 10: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:44:in 'each' 9: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:45:in 'block in render_citations' 8: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:45:in 'sub!' 7: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/asciidoctor/bibliographer_preprocessor.rb:46:in 'block (2 levels) in render_citations' 6: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:36:in 'render' 5: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:71:in 'render_citation_with_csl' 4: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:98:in 'prepare_items' 3: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:98:in 'map' 2: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:98:in 'block in prepare_items' 1: from C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/citation.rb:105:in 'prepare_metadata' C:/tools/ruby25/lib/ruby/gems/2.5.0/gems/asciidoctor-bibliography-0.8.0/lib/asciidoctor-bibliography/database.rb:24:in 'find_entry_by_id': asciidoctor: FAILED: C:/Users/acarver/<blah,blah>/Frame_with_Intro.adoc: Failed to load AsciiDoc document - No entry with id 'Ridderbos1975' was found in the bibliographic database. (AsciidoctorBibliography::Errors::Database::IdNotFound)

ronaldtse commented 6 years ago

@andrewcarver sorry for the late response.

Are we speaking of two problems:

  1. A citation a comment block should not be checked
  2. A bibliographic entry in a BibTeX database is not found despite it being there ?
andrewcarver commented 6 years ago

I guess so. I was not aware, if it's true, that a citation in a comment block should not be checked. In any case, the error arises because 1) it checks this citation, and 2) it says it's not in the datafile, when in actuality it is. Uninstalling 0.8.0 (leaving 0.7.2 as my latest version), and leaving everything else the same, fixes it.

andrewcarver commented 6 years ago

To shed a bit more light:

  1. The problem persists even when all citations are non-commented;
  2. The extension stumbles at whatever happens to be the first citation in the AsciiDoc; the particular citation is unimportant, all that matters is that it's the first one.
andrewcarver commented 6 years ago

By the way, I HOPE that processing of citations within comments is a FEATURE, not a BUG! I really like it!!

andrewcarver commented 6 years ago

I can tell you where the problem is arising for me; but, I'm not yet sure WHY it is:

It's in bibliographer_preprocessor.rb, in the glob_pattern method: the line Dir.chdir(base_dir) { Dir.glob(pattern) } is returning an empty array, even though the parameters the method received seem correct.

andrewcarver commented 6 years ago

More specifically, the Dir.glob(pattern) bit is returning an empty array.

andrewcarver commented 6 years ago

And the problem was in my pattern--to which Dir.glob() took offense, regarding which way my slashes were leaning:

My pattern was ..\BibTeX_libraries\Biblical.bib Dir.glob wants: ../BibTeX_libraries/Biblical.bib

andrewcarver commented 6 years ago

So--the question is, should we force the Windows users to lean their slashes forward? Or is their some nice code-emendation that would catch that kind of pattern, and change the direction the user's slashes lean?

andrewcarver commented 6 years ago

Here's a simple (perhaps too simple) fix:

Dir.chdir(base_dir) { Dir.glob(pattern.gsub( /\\/, '/')) }

That fixes my Windows-backslashes pattern... If you think that's a good idea (to start with, anyway), I can take this over to a new PR, and we can haggle over it there...

andrewcarver commented 6 years ago

Uh-oh--THAT won't work: Dir.glob() allows '\' as an escape character... "Note that this means you cannot use backslash on windows as part of a glob, i.e. Dir["c:\foo*"] will not work, use Dir["c:/foo*"] instead." <ouch> ( http://ruby-doc.org/core-2.5.0/Dir.html )

andrewcarver commented 6 years ago

Either we force the Windows users to lean their slashes forward, then; or, we force people not to escape metacharacters in their globs; or, ...??

paolobrasolin commented 5 years ago

Thanks @andrewcarver, your analysis is on point.

The problem here is that we'd like to allow globs (that's a must, I think), but Dir.glob does not like \. A possible solution is in #98 if you want to give it a spin. In essence, we can replace File::ALT_SEPARATOR (that is, \) only when the constant is defined (that is, on Win* systems). I think this is the solution with the leas amount of assumptions.

However, this brought to my attention another issue: we're using the space character as filename separator for bibliography-database. Technically, that's a valid filename character for a filename. I'd rather put a warning in the readme saying to avoid it than write even more ad-hoc parsing. @ronaldtse I think that filenames containing spaces have not emerged as a common use case, so that should be enough.

I'm afraid that checking citations in block comments was a bug (fixed in 0.9.0, see #75). I'm also not sure why that would be a desirable feature. How were you using it @andrewcarver ?

andrewcarver commented 5 years ago

I have now and then succumbed to the temptation to put relevant items in my bibliography that I've not actually cited anywhere -- but have read for research-purposes, and think them worth citing. (I believe that some other researchers are known to do this on occasion -- though it is forbidden in some scholarly contexts.) The only way to do this with asciidoctor-bibliography, of which I'm aware, was to cite the work(s) in a comment.

ronaldtse commented 5 years ago

@paolobrasolin is there anything file “invalid” character we can rely on than a space? There are people who use this on Windows (plenty on Metanorma) that I am aware the issue will come up. As long as we have some way of handling spaces in filenames/directories, it’ll be fine.

paolobrasolin commented 5 years ago

@andrewcarver we got you covered, then! A couple of days ago version 0.10.0 introduced nocite (see #97 and the readme) which allows you to do exactly that!

paolobrasolin commented 5 years ago

@ronaldtse I wouldn't go down that road. Reasoning about inter-os compatibility and validity of file names is a mess. Suffice it to say that the only two invalid characters (at filesystem level) in *nix are NUL and /. All others are valid and usually escaped only to let the shell interpret them literally.

Furthermore, I think that we need something that's both easy to read in the code and usable from the command line: setting bibliography-database from the cli should not be a headache. For this reason delimiting filenames using single/double quotes doesn't sound good to me (once you need to interpolate some parameters on the CLI, quote nesting/escaping would be nasty).

Escaping literal (= non-delimiting) spaces looks like the cleanest solution to me. In fact, it's exactly what the shell does when handling filenames containing spaces.

# single file
:bibliography-database: foo.bib
asciidoctor ... -a bibliography-database=foo.bib

# multiple files
:bibliography-database: foo.bib bar.bib
asciidoctor ... -a bibliography-database='foo.bib bar.bib'
asciidoctor ... -a bibliography-database=foo.bib\ bar.bib # possible, wouldn't recommend

# filename contains space
:bibliography-database: the\ foo.bib
asciidoctor ... -a bibliography-database='the\ foo.bib'
asciidoctor ... -a bibliography-database="the\ foo.bib" # allows shell interpolation

# filename begins with space, contained in folder
:bibliography-database: the\\ foo.bib
asciidoctor ... -a bibliography-database='the\\ foo.bib'
asciidoctor ... -a bibliography-database="the\\\ foo.bib" # allows shell interpolation

As to whether this pattern would make sense on a win* shell, I'm still not 100% sure as I don't have one handy right now.