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

Fix --sort-fields CLI output #404

Merged
merged 2 commits into from Mar 24, 2023
Merged

Fix --sort-fields CLI output #404

merged 2 commits into from Mar 24, 2023

Conversation

TobiKattmann
Copy link
Contributor

Hi,

first of all thank you for this tool, really nice to quickly interactively test it before buying into it 👍

The CLI output shows --sort_fields with an underscore where it has to be --sort-fields with a dash/minus, i.e. the command from the CLI wont work or to be more precise: error with Unknown option: --sort_fields

image

I searched the repo for --sort_fields, fixed the instance under ./src and ran npm run build.

I wanted to run the docker instructions locally but ran into a

 [ERROR] fetch is not defined [plugin google-font-loader]

during Step 11/17 : RUN npm run build (note it ran fine as a standalone command in the repo). I have absolutely no clue of the languages etc so I stopped trying to get it to run after 15min or so. I also only really found this very recent related github issue. I am on EndeavourOS with node --version = v19.8.1 in case

In the contributing guidelines it says one should run npm build which fails on my system

[tobi@tobiT480 bibtex-tidy]$ npm build
Unknown command: "build"

but npm run build works. So either that has to be changed or it is some version-dependent thing.

Let me know whether this PR is fine :)

@FlamingTempura
Copy link
Owner

Thanks for the fix and feedback on the build process.

The docker issue is due to the container using an old node version (v16) that does not support fetch. Fetch was introduced in v18. I'll update the docker file. Thanks for reporting it!

I'll also update the contributing docs, I think that was just a mistake when I was writing it up.

@FlamingTempura FlamingTempura merged commit c0f2d80 into FlamingTempura:master Mar 24, 2023
8 checks passed
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.

None yet

2 participants