feat: Insert Missing Comma on Enter

[AgentRBY]

Fix #5

P.s.: I have not written extensions in VS Code before, so I am open to suggestions

[zardoy]

Wow looks interesting! I think current implementation is fine enough, but probably only for json without comments. Initially I was thinking of using some parsing method to insert comma more precisely, some cases to test:

I marked with | positions where comma would be inserted incorrectly.

{
  "key": "value" /* comment */ // case 1|
}

Btw I checked that WebStorm doesn't do this auto comma inserting after comments and don't think its right.

I think we can easily workaround this comments problem just by proceeding the editor text with strip-json-comments module.

Btw I see I didn't check this project for a long time, so I'll try to setup a decent CI soon.

[AgentRBY]

Thx for feedback! I fixed everyting exept startsWith because regex is very slow.

I think we can easily workaround this comments problem just by proceeding the editor text with strip-json-comments module.

Also added JSONC support with "strip-json-comments"

In multicursor mode i have some problems:

1. prevLinePosition did not point to the last character, so I create a new Position
2. In some cases the comma is not inserted (see gif) and IDK how fix that

[zardoy]

Also what about case that I mentioned above:

{
    "key": "value" //comment
}

I think its still would be good to handle cases like this (insert comma after "value"). What do you think?

I think these requested changes are final, just need to figure out why CI checks don't run on this pull request? 🤔 Maybe you need to reopen the pr?

[AgentRBY]

Also what about case that I mentioned above:

{
    "key": "value" //comment
}

I think its still would be good to handle cases like this (insert comma after "value"). What do you think?

Done.

I think these requested changes are final, just need to figure out why CI checks don't run on this pull request? 🤔 Maybe you need to reopen the pr?

Maybe, IDK. After all the issues are resolved, I will try to close this PR and create a new one

[zardoy]

Tried to test this:

{
    "key": "value" //comment|
}

When pressing Enter at | position comma still doesn't get inserted after "value" e.g.:

{
    "key": "value", //comment
    |
}

You might want to get prevLineLastChar from textWithoutComments. Don't forget to pass trailingCommas: false as an option for stripJsonComments.

And I also don't understand the purpose of isCurrentLineBeforeComment.

[AgentRBY]

When pressing Enter at | position comma still doesn't get inserted after "value"

Oh, I misunderstood what you want to do a little, but I still implemented another good fix (see gif below)

And I also don't understand the purpose of isCurrentLineBeforeComment.

[AgentRBY]

When pressing Enter at | position comma still doesn't get inserted after "value" e.g.:

Also, I implemented your proposal

[zardoy]

I'll try to add & push integration tests soon (probably tomorrow)

[AgentRBY]

What do you think about the case when pasting string: Do we have to insert a comma in this case?

[zardoy]

Do we have to insert a comma in this case?

I don't think its a common case and let's not do this, because the setting description explicitly says that we do this only after Enter, so this behavior after pasting can be surprising & inconsistent to user. Futhermore user might not know wether just copied string starts with newline and I can imagine a more common case where user hits Enter and then pastes copied JSON content. That's why I was requesting that change above.

[zardoy]

Hey, @AgentRBY I've pushed integration tests and also fixes for them. Can you please wether they are good for you? I tried to to cover only basic cases, but probably there are any other cases that you would like to cover?

UPD: Also I removed the check that current line starts with comment. I didn't remember what cases it was covering? Gonna be honest, I don't think we ever need startsWithComment function here.

Meanwhile I'll try to fix CI issues here.

[AgentRBY]

Hey, @AgentRBY I've pushed integration tests and also fixes for them. Can you please wether they are good for you? I tried to to cover only basic cases, but probably there are any other cases that you would like to cover?

I've added some other simple tests to test more cases Also, I think we should add tests for the pasting after comment (as in gif): But I haven't figured out how to do it, so I'll leave it to you, @zardoy

UPD: Also I removed the check that current line starts with comment. I didn't remember what cases it was covering? Gonna be honest, I don't think we ever need startsWithComment function here.

It was needed rather for optimization, in order to immediately discard values ​​that begin with comments. In general, its removal does not harm anything.

[zardoy]

Also, I think we should add tests for the pasting after comment

I was going to add them as well, but forgot about, thanks for pointing!

[zardoy]

@AgentRBY I misread your comment:

I've added some other simple tests to test more cases Also, I think we should add tests for the pasting after comment (as in gif):

I didn't get the idea about what you were talking, as I can see in the gif, you're just doing ctrl+Enter or Enter at the end. We already have test for this: https://github.com/zardoy/vscode-fix-all-json/pull/6/files#diff-cfd18f0b3c283dd23cf5513db00647d1949112658b52c7f406359cc92acdec00R19

[AgentRBY]

I didn't get the idea about what you were talking, as I can see in the gif, you're just doing ctrl+Enter or Enter at the end. We already have test for this: https://github.com/zardoy/vscode-fix-all-json/pull/6/files#diff-cfd18f0b3c283dd23cf5513db00647d1949112658b52c7f406359cc92acdec00R19

Okay, I misunderstood the test you pointed out a bit. I just checked and it's really what I wanted

I think it's ready to merge @zardoy

[zardoy]

@AgentRBY Thank you so much for the idea & implementation 🎉. I'm happy that now I have solution when editing JSON in the web. Do you want to handle a similar problem: https://github.com/zardoy/vscode-fix-all-json/issues/9 ?

[AgentRBY]

@AgentRBY Thank you so much for the idea & implementation 🎉. I'm happy that now I have solution when editing JSON in the web. Do you want to handle a similar problem: #9 ?

Okay i will do it

Also check #10, if you like the idea, I will implement it too