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

Fixed a bug where saving an empty item, makes a new line in the INI file. #2

Merged
merged 3 commits into from
Apr 17, 2015
Merged

Conversation

Ahmad45123
Copy link
Contributor

More INFO: http://forum.sa-mp.com/showpost.php?p=3433253&postcount=13

And also I removed a useless copy of writing.inc file.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

I said I would help people out with learning the YSI internals for a little bit to help the transition.

That code will fix the general case, but y_ini supports comments, and the naming of "e_INI_LINE_TYPE_BLANK" is a bit bad, it should maybe be "e_INI_LINE_TYPE_DATALESS". A "blank" line has no tags or key/value pairs, but that doesn't actually mean it is totally empty, as a comment on its own line counts as "blank":

[tag]
name = value
; This line is "blank"
key = value

With that fix (I think), your file will end up like this after a save:

[tag]
name = new_value
key = new_value

You've lost the comments.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

A better fix would possibly be:

if (!deleted && p2s != p2e) fwrite(buffer, sLine);

There are several returns from INI_IdentifyLineType - p0, p1, and p2, each with a start (s) and end (e). p0s is the start of the first line part - either the tag name or the current key depending on the line type, p0e is the end. p1x are the start and end of the second line part - either the tag parent or the current value. p2x is the third line part, i.e. the comments. If any part is missing, then pNs == pNe.

@Ahmad45123
Copy link
Contributor Author

About POST 1:
I don't really get you.. What was happening is that when saving an item multiple times it was creating new line everytime which made the ini file very huge...

But now it only creates one empty line. (I don't really use comments if you are saying that it will remove the comments there.)

And about POST 2: Thanks for the info, I will give it a try.

@Misiur
Copy link
Collaborator

Misiur commented Apr 17, 2015

Yup, I read through writing.inc and came to same conclusion e_INI_LINE_TYPE_BLANK naming is misleading. @johnymac: You want to update your PR to reflect that?
I will rename that to ...DATALESS in next commit.

@Ahmad45123
Copy link
Contributor Author

Ok, It made the same result as mine.
Can someone please expain why this one is better ? I cant get it.

And yes I will commit it now.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

It is better because even if you don't use comments, that doesn't mean no-one does. So either fix will work for you, because you have no comments to preserve, but anyone with comments will loose them.

@Misiur
Copy link
Collaborator

Misiur commented Apr 17, 2015

The best solution would be if there wasn't a duplicate new line created (I've tried additional conditions around line 923), but it get's really weird when the value is empty, but there is a comment in the same line. We'll stick to this fix for foreseeable future.

Misiur added a commit that referenced this pull request Apr 17, 2015
Fix duplicate new lines, when key has empty value.
@Misiur Misiur merged commit 2ff8c1b into pawn-lang:YSI.tl Apr 17, 2015
@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

Change lines 918-920 to:

                            fwrite(buffer, YSI_g_sINIWriteBuffer[curSlot][E_INI_KV_ENTRY_NAME]),
                            fwrite(buffer, " = "),
                            fwrite(buffer, YSI_g_sINIWriteBuffer[curSlot][E_INI_KV_ENTRY_TEXT]);

The old version (I think, I've not tested any of this), printed the old version of the line, ending at the start of the value (p1s), then printed the new value. However, because there was no old value it got confused and somehow a new line was appended (I suspect p1s is set to the end of the line). This version just forgets about the old key and writes the new key, which happens to be identical.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

If I'm right about that fix, it should never write the extra blank line in the first place, so the old fix can be reverted. With that other fix, explicit blank lines are also consumed, so a neatly laid out INI file like:

[tag_1]
key_x = 8
key_y = 9

[tag_2]
key_x = 11
key_y = 6

Will become:

[tag_1]
key_x = 8
key_y = 9
[tag_2]
key_x = 11
key_y = 6

Which IMHO is unlikely to be what is wanted.

@Ahmad45123
Copy link
Contributor Author

Ok, I did some fast testing and the old version didn't add spaces between tags also..
That feature isn't even there... :P

So I think using the second solution would be better.

@Misiur
Copy link
Collaborator

Misiur commented Apr 17, 2015

Ok, almost perfect. Currently when comment is after empty value the ";" get's copied ad infinitum. For some reason p2s is at ; character. When value is present, the p2s is on first character of the comment after ;. Will report shortly.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

I'm not saying it ADDED spaces, I'm saying it didn't REMOVE them. If you have a hand written/edited INI file that has spaces anywhere (that example was just between the tags), then they are preserved in the old version to match the user's style as closely as possible. That's why the old write line code used sLine instead of E_INI_KV_ENTRY_NAME to write the key - if you wrote "key=value", it would write "key=new_value". If you wrote "key = value", it would write "key = new_value", but preserving that AND fixing the bug is hard. You will notice that there's no way in y_ini to write comments either, but they are still preserved because they code goes to some lengths to ensure that it doesn't clobber what a user has written.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

That comment location bug is interesting - p2s should be consistent regardless of what came before, but I can see why that wouldn't be the case in different states.

@Ahmad45123
Copy link
Contributor Author

I was doing some more tests and that bug isn't happening to me :/
I have a comment before and after an empty item.
And when I resaved it multiple times.. The comment doesn't change at all.

@Misiur
Copy link
Collaborator

Misiur commented Apr 17, 2015

Hah, this is quite peculiar one:

Test = ;something
Test2 =;something else
Test3 =  ;what on earth

Gets rewritten as

Test = ; something
Test2 =  ; ; something else
Test3 =  ; ; what on earth

So, when empty value is written we have two padding spaces - one from " = ", other one from INI_WriteComments, and this shakes stuff up a little bit.

@Misiur
Copy link
Collaborator

Misiur commented Apr 17, 2015

Misiur@a447167 fixes comment duplication as well.

@Y-Less
Copy link
Member

Y-Less commented Apr 17, 2015

I was literally typing that line 599 fix as you posted that. You may also want to change INI_WriteComments to use:

        fwrite(buffer, ";"),     \

To avoid all the extra spaces.

DEntis-T added a commit to bracesoftware/YSI-Includes-Doc that referenced this pull request Jul 31, 2023
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.

3 participants