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

New parsebib.el version. #849

Open
joostkremers opened this issue Nov 2, 2024 · 6 comments
Open

New parsebib.el version. #849

joostkremers opened this issue Nov 2, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@joostkremers
Copy link

joostkremers commented Nov 2, 2024

Hi @bdarcus, I've taken the recent issue with post-processing of field values as incentive to finally do what I've been wanting to do for a long time: replace the horrible regexp-based parsing code in parsebib.el with something that at least somewhat resembles a real parser.

I've now gone and done that: I implemented a simple, custom recursive descent parser to replace the regexp-based code. It passes all tests, and I can open my main bibliography file in Ebib without issues, but test coverage isn't all that great and my own bibliography files are created by Ebib, so they're not a real test.

So I was wondering if you have some test file or something that you'd be willing to share, or test yourself.

And more generally, what would it take for you to be comfortable with me pushing the new parsing code to master? The high-level API hasn't changed, so there should be no need for any changes in Citar, but since there appear to be quite a lot of people that rely on Citar, I don't want to cause unnecessary trouble.

BTW, the new version of parsebib.el currenly lives at https://github.com/joostkremers/parsebib/tree/wip/rdp.

@joostkremers joostkremers added the enhancement New feature or request label Nov 2, 2024
@bdarcus
Copy link
Contributor

bdarcus commented Nov 2, 2024

Hi @joostkremers - thanks for checking! I'll copy @roshanshariff in case he has anything to contribute.

There is a small test file in the repo, but I don't think that'd be very helpful.

I did just compile and load the file, and got the below warnings, which go away if I compile and load it a second time. Not sure how significant they are, though ideally they wouldn't bother users.

Citar seems to run fine regardless on a quick check.

Compiling file /home/bruce/Downloads/parsebib.el at Sat Nov  2 14:25:06 2024

In parsebib-find-next-item:
parsebib.el:443:8: Warning: function parsebib-find-next-item used to take 0 or
    1 arguments, now takes 0

In parsebib-read-entry:
parsebib.el:469:8: Warning: function parsebib-read-entry used to take 1-5
    arguments, now takes 0-3

In parsebib-read-string:
parsebib.el:501:8: Warning: function parsebib-read-string used to take 0-2
    arguments, now takes 0 or 1
parsebib.el:512:39: Warning: ‘parsebib--expand-strings’ called with 3
    arguments, but accepts only 2

In parsebib--post-process:
parsebib.el:545:35: Warning: ‘parsebib--expand-strings’ called with 3
    arguments, but accepts only 2

In parsebib--expand-strings:
parsebib.el:551:8: Warning: function parsebib--expand-strings used to take 2
    arguments, now takes 3

@joostkremers
Copy link
Author

There is a small test file in the repo, but I don't think that'd be very helpful.

Well, just to be certain, I'll run the tests anyway.

I did just compile and load the file, and got the below warnings, which go away if I compile and load it a second time. Not sure how significant they are, though ideally they wouldn't bother users.

They are expected, since I changed the signature of some of the functions. But they're all functions that Citar doesn't use directly, so apart from the warnings, which will probably pop up once users upgrade, nothing bad should happen.

Citar seems to run fine regardless on a quick check.

That's good to know, thanks for checking.

I'll wait and see if @roshanshariff has anything, and I'll be hunting down some random BibTeX files from the web, to see how parsebib.el handles them, and I'll give you a heads-up in this issue when I feel confident enough to push the new version to master.

@joostkremers
Copy link
Author

I just pushed the new parser to Github. It should start appearing in Melpa soon.

@bdarcus
Copy link
Contributor

bdarcus commented Nov 9, 2024 via email

@bdarcus
Copy link
Contributor

bdarcus commented Nov 9, 2024 via email

@joostkremers
Copy link
Author

What's the version number for this?

It's version 6.0 in the Version: header. I haven't tagged it yet, because AFAIU that would push it to Melpa Stable as well, and I think it might be better to wait a bit with that.

For Citar, though, the version shouldn't matter, because the parsebib functions that Citar uses haven't changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants