Skip to content

Conversation

@Freed-Wu
Copy link
Contributor

Add a github workflow to check if a json is sorted

Copy link
Owner

@lervag lervag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll have to check this before accepting. You can also check. I would actually not be surprised if this now fails at \faFile*.

Can you explain the github workflow for me? I'm not an expert on that, and I'm curious if we should make it more specific? I.e., now the workflow triggers on all json files, but that does not seem correct if there should be more json files added?

@lervag lervag mentioned this pull request Oct 24, 2022
@Freed-Wu
Copy link
Contributor Author

Freed-Wu commented Oct 24, 2022

Can you explain the github workflow for me

          for json in assets/json/*.json ; do
            diff <(jq -S . $json) $json || exit 1
          done

if a json is not sorted, jq -S . will sort it and print the sorted json to
stdout, so diff the stdout and original json will return 1, || exit 1 will
quit the for-loop, Like this:

screenshot

@lervag
Copy link
Owner

lervag commented Oct 24, 2022

Thanks. As I wrote above:

I'll have to check this before accepting. You can also check. I would actually not be surprised if this now fails at \faFile*.

I've checked, and as you see, it now fails:

image

The reason is that the Vim dictionary does not preserve a sorted order, necessarily. So, this means we must either sort the item(DICTIONARY) or store the data as a sorted list in the json file.

Regarding the workflow: I'm still wondering if it is a good idea to trigger this workflow on any json file and to specify that all json files must be sorted? Would it not be more appropriate to restrict it to this specific file?

@Freed-Wu
Copy link
Contributor Author

Would it not be more appropriate to restrict it to this specific file?

Fixed.

store the data as a sorted list in the json file.

Fixed.

Add a github workflow to check if a json is sorted
@lervag
Copy link
Owner

lervag commented Oct 26, 2022

I did some minor profiling, and fascinatingly, doing the json_decode(fileread()) is faster than inlining in vimscript.

I'm merging this now. Thanks for contributing!

lervag added a commit that referenced this pull request Oct 26, 2022
@lervag lervag merged commit 0730c35 into lervag:master Oct 26, 2022
@lervag
Copy link
Owner

lervag commented Oct 26, 2022

I made a few minor changes to the workflow (https://github.com/lervag/vimtex/blob/master/.github/workflows/assets.yml), mostly that I fixed the path to the fontawesome file. That will avoid any surprises if I were to add more json files to the assets directory. I believe I should probably collect all assets within that subdirectory, so I might look into that in the not so far away future.

@lervag
Copy link
Owner

lervag commented Oct 26, 2022

Hmm. For some reason, the updated workflow does not run properly now:

https://github.com/lervag/vimtex/actions/runs/3332974138

Perhaps you could assist me here? Here's the diff of the workflow file from your version to the current:

image

@Freed-Wu
Copy link
Contributor Author

Fixed 👍

@lervag
Copy link
Owner

lervag commented Oct 27, 2022

Thanks! I'll look into the new PRs later today!

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.

2 participants