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

Unescaping works not as intended and is slow #296

Closed
Totto16 opened this issue Jun 4, 2024 · 0 comments · Fixed by #297
Closed

Unescaping works not as intended and is slow #296

Totto16 opened this issue Jun 4, 2024 · 0 comments · Fixed by #297
Assignees
Labels
bug Something isn't working Priority High Priority High

Comments

@Totto16
Copy link
Collaborator

Totto16 commented Jun 4, 2024

Description

After I made #266 and tested it, it passed all manual tests and also did perform normally. But after using it locally on real data it performs poorly, taking seconds to unescape everything in my history (I have a 500 entries history 😓 )

After investigating this issues, and reading into the source code of default_unescape_string I found out, that they unescape '' (two single quotes) to ' and tested the input of '' and copying that entry, it was unescaped to ' which is incorrect.

Also the performance problem lies into calling C functions is rather expensive, and doing it for ~500 entries takes a few seconds, even if the c function is fast, the transition costs to much in this case.

The best solution is to rewrite the exact needs of the unescape behavior ourselves in js, even if I wonder why this problem exists in the first case in gda.

I am working on fixing this 👍🏼

@Totto16 Totto16 added bug Something isn't working Priority High Priority High labels Jun 4, 2024
@Totto16 Totto16 self-assigned this Jun 4, 2024
@Totto16 Totto16 linked a pull request Jun 4, 2024 that will close this issue
8 tasks
Totto16 added a commit that referenced this issue Jun 4, 2024
- bug #296
- this is a backport of PR #297
@Totto16 Totto16 mentioned this issue Jun 4, 2024
8 tasks
Totto16 added a commit that referenced this issue Jun 4, 2024
- this is a backport of PR #297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Priority High Priority High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant