Skip to content

jq.lua: Use temp_file strategy for non-Win32 machines #3

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

jaredbarranco
Copy link

This method correctly handles single quotes and allows for better debugging of the plugin when users have questions. If a method/function errors out the ~/.local/share/nvim/lazy/json-nvim/temp.json or other PLUGIN_HOME dir will have the "in-progress" file.

use temp_file for non-windows is_valid

debug is_valid

back to original

debug loggers

more debug loggers

fix debug

log exit_status

remove redundant debugs, add validity check debug

bool to string

debugging

debugging

debugging

debugging
In jq.lua, use temp file for jq actions.

remove debug line
Use temp file for non-windows jq actions
@jaredbarranco
Copy link
Author

Because of issues related in Issue #2

@jaredbarranco jaredbarranco marked this pull request as ready for review February 16, 2025 23:26
@jaredbarranco
Copy link
Author

@VPavliashvili I did some preliminary testing with some of the functions I am commonly using.

I'll be making another PR in the next couple of weeks for some additional improvements if you're open to it.

@VPavliashvili
Copy link
Owner

VPavliashvili commented Feb 17, 2025

@jaredbarranco I started to run tests too and found some problems
for example take this json(generated from online generator)

[
  {
    "_id": "67b3739c332882c1456e076b",
    "index": 0,
    "guid": "9a249e2b-173b-4ab9-ac77-9257d780eedb",
    "isActive": true,
    "balance": "$3,456.75",
    "picture": "http://placehold.it/32x32",
    "age": 28,
    "eyeColor": "green",
    "name": "Olga Hess",
    "gender": "female",
    "company": "REPETWIRE",
    "email": "[email protected]",
    "phone": "+1 (920) 506-2554",
    "address": "918 Fillmore Avenue, Chapin, Indiana, 8526",
    "about": "Proident ea enim sit commodo ullamco minim sint non. Cupidatat reprehenderit pariatur sit in in pariatur reprehenderit consectetur occaecat sit. Consectetur excepteur nisi adipisicing pariatur ullamco labore voluptate elit in do.\r\n",
    "registered": "2016-07-23T05:21:41 -04:00",
    "latitude": -38.870975,
    "longitude": 139.528881,
    "tags": [
      "nisi",
      "aliquip",
      "exercitation",
      "id",
      "velit",
      "eiusmod",
      "proident"
    ],
    "friends": [
      {
        "id": 0,
        "name": "Constance Mcmillan"
      },
      {
        "id": 1,
        "name": "Alexandria Lamb"
      },
      {
        "id": 2,
        "name": "Amparo Barber"
      }
    ],
    "greeting": "Hello, Olga Hess! You have 8 unread messages.",
    "favoriteFruit": "strawberry"
  }
]

When I tried to format after your PR I got this result (run :JsonMinifyFile)

[  {    "_id": "67b3739c332882c1456e076b",    "index": 0,    "guid": "9a249e2b-173b-4ab9-ac77-9257d780eedb",    "isActive": true,    "balance": "$3,456.75",    "picture": "http://placehold.it/32x32",    "age": 28,    "eyeColor": "green",    "name": "Olga Hess",    "gender": "female",    "company": "REPETWIRE",    "email": "[email protected]",    "phone": "+1 (920) 506-2554",    "address": "918 Fillmore Avenue, Chapin, Indiana, 8526",    "about": "Proident ea enim sit commodo ullamco minim sint non. Cupidatat reprehenderit pariatur sit in in pariatur reprehenderit consectetur occaecat sit. Consectetur excepteur nisi adipisicing pariatur ullamco labore voluptate elit in do.\r\n",    "registered": "2016-07-23T05:21:41 -04:00",    "latitude": -38.870975,    "longitude": 139.528881,    "tags": [      "nisi",      "aliquip",      "exercitation",      "id",      "velit",      "eiusmod",      "proident"    ],    "friends": [      {        "id": 0,        "name": "Constance Mcmillan"      },      {        "id": 1,        "name": "Alexandria Lamb"      },      {        "id": 2,        "name": "Amparo Barber"      }    ],    "greeting": "Hello, Olga Hess! You have 8 unread messages.",    "favoriteFruit": "strawberry"  }]

note extra spacing, I think this is a regress.
before your commit it looks like this:

[{"_id":"67b3739c332882c1456e076b","index":0,"guid":"9a249e2b-173b-4ab9-ac77-9257d780eedb","isActive":true,"balance":"$3,456.75","picture":"http://placehold.it/32x32","age":28,"eyeColor":"green","name":"Olga Hess","gender":"female","company":"REPETWIRE","email":"[email protected]","phone":"+1 (920) 506-2554","address":"918 Fillmore Avenue, Chapin, Indiana, 8526","about":"Proident ea enim sit commodo ullamco minim sint non. Cupidatat reprehenderit pariatur sit in in pariatur reprehenderit consectetur occaecat sit. Consectetur excepteur nisi adipisicing pariatur ullamco labore voluptate elit in do.\r\n","registered":"2016-07-23T05:21:41 -04:00","latitude":-38.870975,"longitude":139.528881,"tags":["nisi","aliquip","exercitation","id","velit","eiusmod","proident"],"friends":[{"id":0,"name":"Constance Mcmillan"},{"id":1,"name":"Alexandria Lamb"},{"id":2,"name":"Amparo Barber"}],"greeting":"Hello, Olga Hess! You have 8 unread messages.","favoriteFruit":"strawberry"}]

please consider finding the cause and fixing it
EDIT: I should have specified that I run this test on Linux

@jaredbarranco
Copy link
Author

jaredbarranco commented Feb 19, 2025

@VPavliashvili Thanks for catching that regression. I believe I corrected it with this commit.

I need to do some additional regression testing before I can confidently say it's ready to go.

On a related note, I'm going to open an issue for discussion around testing framework - something I think would be helpful for making sure regression like this one doesn't happen in the future.

@VPavliashvili
Copy link
Owner

@jaredbarranco Do you think it's better to incorporate unit tests first and then proceed with this PR or other way around?

@jaredbarranco
Copy link
Author

@VPavliashvili Unit testing first would be best so that we can get a baseline expectation. If possible, I think you should be the one to generate those because I can't run ANY of the functions with master from your repo on Mac and everything but the JsonKeysTo* functions on linux.

@VPavliashvili
Copy link
Owner

VPavliashvili commented Feb 19, 2025

@jaredbarranco unfortunately I am on a tight schedule right now and you will have to wait for a little while before I will be able to do the work, otherwise I am down to your proposition.
Also before that, if you provide some small example of some tests, it would be nice and helpful(ofc not required tho)

EDIT: Lets continue unit testing related discussion in #4 and leave this section specific to the PR

…read

move json-nvim to json_nvim

move lua/json-nvim to lua/json_nvim

not including files that were update for string input testin

add M. functions that accept a string for operations

minify, format, and escape - accept a string

working test script with input/goldens

delete un-used lua/json_nvim/json-nvim.lua

Change name from Token to Node

github actions workflow
echo runner temp

cache apt, env. namespace incorrect

fix tmpdir declaration

set temp for windows
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