analytic-bias commented 1 year ago

Description

What is the bug?

Repeated backslash escape

Problem Explanation

Write a clear and concise description of what the bug is:

The bug happens when one tries to copy a text with backslashes.

Expected Behavior

A clear and concise description of what you expected to happen, and how the actual outcome differs:

Pano shouldn't repeatedly apply character backslash escapes, as in
It should behave correctly when copying and pasting text with backslash characters.

Reprodution

How one can find the bug?

Steps To Reproduce

Steps to reproduce, if applicable:

Copy this text:
```
\( X^*(T)^{\vee} = \mathbb{Z}^r \)
```
Open the clipboard history.
Optionally repeat

Details

Mark with [ ] all that applies:

It happens with any application?

[x] Yes, it applies to any application.
[ ] No. Only with the following applications:
- VSCode
[ ] It works with the following applications that I have tried:
- Fill in a list with any application that applies

It happens only on one computer?

[x] I don't know.
[ ] No. I have tried it on more than one computer.

It happens only with some specific gnome configuration?

[x] I don't think that the configuration matters.
[ ] Yes. Only if the following config is set up:
- Fill in a list with any configuration tha applies.

It happens only with some specific extension installed?

[x] I don't think that the installed extensions affect the bug/behavior.
[ ] Yes. Only if the following gnome extension is installed:
- Fill in a list with any extension tha applies.
- Fill in also any detail about the extensions that applies.

Diagnostics

Under what conditions does it happen?

Fill in all information that applies:

Environment

Totto16 commented 8 months ago

Duplicate (but still not fixed) of #107

e2ipi-1 commented 5 months ago

Here's the method I'm using to overcome this problem while awaiting a more serious repair from the developers, and which gives good results with the "Ubuntu 23.10/GNOME 45.2/Pano 22" configuration (for Ubuntu 22.04, see remark 1 below).

The idea is to externally restore the pano.db database after each modification made by the extension.

In the following, <USER> refers to the user name.

Packages required

incron

sudo apt install incron

sqlite3

sudo apt install sqlite3

Method

1. Modify the ~/.local/share/gnome-shell/extensions/pano@elhan.io/extension.js file (after saving it) according to the attached extension.js.diff.txt file. You can place this file in ~/.local/share/gnome-shell/extensions/pano@elhan.io and run:

cp extension.js extension.js.bak

then

patch -i extension.js.diff.txt extension.js

extension.js.diff.txt

2. Delete the ~/.local/share/pano@elhan.io folder. Note that all clipboard contents will be lost.

3. Restart session.

4. The ~/.local/share/pano@elhan.io folder has been recreated and the pano.db database it contains shows a new changeType column added at the end, which will be consulted after each modification of the database by the extension. This column can contain one of the following 3 values: 0 for a new entry; 1 for an existing entry that has just been modified by the extension; 2 after the database has been modify externally by sqlite3, as we shall see.

5. Create a script ~/.local/bin/pano-modify_database.sh and make it executable:

#!/bin/bash

sleep 0.1

cd /home/<USER>/.local/share/pano@elhan.io

sqlite3 pano.db << EOF
UPDATE clipboard SET content = REPLACE(content,'\\\\','\\') WHERE changeType = 0 OR changeType = 1;
UPDATE clipboard SET matchValue = REPLACE(matchValue,'\\\\','\\') WHERE changeType = 1;
UPDATE clipboard SET searchValue = REPLACE(searchValue,'\\\\','\\') WHERE changeType = 1;
UPDATE clipboard SET changeType = 2;
EOF

incrontab -d

6. Finally, configure incron: type

sudo nano /etc/incron.allow

then type <USER> then Ctrl+S and Ctrl+X then incrontab -e then type

/home/<USER>/.local/share/pano@elhan.io/pano.db IN_MODIFY,IN_ONESHOT /home/<USER>/.local/bin/pano-modify_database.sh

then Ctrl+S and Ctrl+X.

Remarks

1. This method also works well with the "Ubuntu 22.04.3/GNOME 42.9/Pano 19" configuration. You can use the following file to patch extension.js:

pano19-extension.js.diff.txt

2. Not familiar with GJS, the changes made to the extensions.js file dealing with the new changeType column are certainly not optimal. Let's just say that it seems to work like that for my use of Pano, any improvements are welcome!

3. The contents of the database can be viewed at any time via

sqlite3 ~/.local/share/pano@elhan.io/pano.db

then

SELECT * FROM clipboard;

Warning about clearing history

When history is cleared, the ~/.local/share/pano@elhan.io folder is deleted and the incron table must be reloaded with incrontab -d once this folder has been recreated.

Note that clearing history seems to be a problem with "Ubuntu 23.10/Gnome 45.2/Pano 22" configuration: the ~/.local/share/pano@elhan.io folder is deleted but not recreated...

And with "Ubuntu 22.04.3/Gnome 42.9/Pano 19" configuration, the ~/.local/share/pano@elhan.io folder is recreated, but running incrontab -d is not enough, you must restart your session.

Here's how I make it automatic with the "Ubuntu 23.10/Gnome 45.2/Pano 22" configuration, bearing in mind that I only clear the history from the Pano indicator.

1. Modify the ~/.local/share/gnome-shell/extensions/pano@elhan.io/extension.js file again: replace stop() with disable() and start() with enable() in the definition of the clearHistory() function.

2. Restart session.

3. Create a script ~/.local/bin/pano-reload_incrontab.sh and make it executable:

#!/bin/bash

sleep 1
incrontab -d

4. Update incron table to monitor possible deletion of pictures folder contained in ~/.local/share/pano@elhan.io: run incrontab -e then type

/home/<USER>/.local/share/pano@elhan.io/pictures IN_DELETE_SELF,IN_ONESHOT /home/<USER>/.local/bin/pano-reload_incrontab.sh

then Ctrl+S and Ctrl+X.

NinZeige commented 5 months ago

I read the code of Pano and the interface of libgda, and my preliminary guess is that the reason is that the add_field_value_as_gvalue function will automatically escape the backslash (I'm not very clear on the reason).

This is specifically reflected in the db.ts:Database:save() function, where builder.add_field_value_as_gvalue('content', dbItem.content as any) is used.

Since I'm not familiar with ts, I wrote a piece of code with the same logic in C and confirmed this point.

#include <libgda/libgda.h>
#include <stdio.h>

char x[] = "abc\\\ndef";

int main() {
    GdaSqlBuilder *gsb = gda_sql_builder_new(GDA_SQL_STATEMENT_INSERT);
    gda_sql_builder_set_table(gsb, "clipboard");
    GValue gv = G_VALUE_INIT;
    g_value_init(&gv, G_TYPE_STRING);
    g_value_set_static_string(&gv, x);
    gda_sql_builder_add_field_value_as_gvalue(gsb, "content", &gv);
    GError *e;
    GdaStatement *gs = gda_sql_builder_get_statement(gsb, &e);
    printf("%s\n", x);
    printf("%s\n", gda_statement_serialize(gs));
}

And the result of the code is:

abc\
def
{"statement":{"sql":null,"stmt_type":"INSERT","contents":{"table":"clipboard","fields":["content"],"values":[[{"value":"'abc\\\\\ndef'"}]]}}}

When copying text with a backslash for the first time, the data inside the database has already become two backslashes. Since utils/panoItemFactory.rs:findOrCreateDbItem simply returns the data itself rather than reading from the DB, the text initially displayed on Pano is not escaped (this can be proven by shutdown and reopening Pano). But subsequent usage will encounter the problem of continuous escaping.

Totto16 commented 4 months ago

The error is indeed what @NinZeige discovered, thank you for that :heart: And the fix is to just unescape the content on reading it from the db (see #266) , it may have issues, even if I tested it with some \ heavy files (some latex files, it was really annoying to copy things there with this issue 😓 )

@analytic-bias @NinZeige It would really help if you tested it locally, by following the steps in the README ❤️

e2ipi-1 commented 4 months ago

@Totto16 Although I was not invited to do so, I was able to test your proposal.

I made the modifications indicated in the extension.js file. Getting an error on my "Ubuntu 23.10/GNOME 45.2/Pano 22" system, I replaced "Gda5.default_unescape_string" with "Gda.default_unescape_string".

That said, Pano's operation around backslash is improving and looking more like it should, but problems remain. For example:

Copy \test1 then \test2.
Open the panel and paste \test1.
Open panel and paste \test2.
Open panel and paste \test1.
Finally, open the panel and observe a duplicate \test1.

For the time being, I'll continue with the incron DIY I described above, which seems to work better.

Best regards

Totto16 commented 4 months ago

@Totto16 Although I was not invited to do so, I was able to test your proposal.

I made the modifications indicated in the extension.js file. Getting an error on my "Ubuntu 23.10/GNOME 45.2/Pano 22" system, I replaced "Gda5.default_unescape_string" with "Gda.default_unescape_string".

That said, Pano's operation around backslash is improving and looking more like it should, but problems remain. For example:
1. Copy `\test1` then `\test2`.

2. Open the panel and paste `\test1`.

3. Open panel and paste `\test2`.

4. Open panel and paste `\test1`.

5. Finally, open the panel and observe a duplicate `\test1`.
For the time being, I'll continue with the incron DIY I described above, which seems to work better.

Best regards

The name Gda5 just comes from the import name of import Gda5 from '@girs/gda-5.0'; but at transpile time from rollup it gets converted to import Gda5 from 'gi://Gda?version>=5.0'; so that's just a name, that gets associated with gda, since we use 5.0 types here, but support both 6.0 and 5.0

That said, I mentioned you in the Issue #107 and didn't want to mention you twice 😓 😂

I will look into this issue and see, if it can be solved, since the other solution is not that easy to do as a new user and this issue should be fixed in the extension itself, thank you for your testing ❤️

e2ipi-1 commented 4 months ago

Thank you for your message and this clarification about Gda5.

I was able to do some more tests. Taking up your idea around "matchValue" and "searchValue", the situation seems to improve: the problem I mentioned earlier disappears and more generally, the database seems to behave as it should. More precisely, I've modified my extension.js file as follows (in the places indicated):

+ const content_unescaped = Gda.default_unescape_string(content) ?? content;
+ const matchValue_unescaped = Gda.default_unescape_string(matchValue) ?? matchValue;
+ const searchValue_unescaped = Gda.default_unescape_string(searchValue) ?? searchValue;

- content,
+ content: content_unescaped,
- matchValue,
+ matchValue: matchValue_unescaped,
- searchValue,
+ searchValue: searchValue_unescaped,

Acting at the level of the extension itself is of course the right method.

Thank you for your work on Pano, especially on the switch to GNOME 45 and on this major problem. 👍

Totto16 commented 4 months ago

Thank you for your message and this clarification about Gda5.

I was able to do some more tests. Taking up your idea around "matchValue" and "searchValue", the situation seems to improve: the problem I mentioned earlier disappears and more generally, the database seems to behave as it should. More precisely, I've modified my extension.js file as follows (in the places indicated):
+ const content_unescaped = Gda.default_unescape_string(content) ?? content;
+ const matchValue_unescaped = Gda.default_unescape_string(matchValue) ?? matchValue;
+ const searchValue_unescaped = Gda.default_unescape_string(searchValue) ?? searchValue;

- content,
+ content: content_unescaped,
- matchValue,
+ matchValue: matchValue_unescaped,
- searchValue,
+ searchValue: searchValue_unescaped,
Acting at the level of the extension itself is of course the right method.

Thank you for your work on Pano, especially on the switch to GNOME 45 and on this major problem. 👍

Ah yes, that seems a good idea, I already said, that there might be potential other fields, where unescaping is needed, I never save the search value or match value, so it's clear, why I never caught that bug. I'll test that myself later 👍🏼 Thank you too, for testing this ❤️

e2ipi-1 commented 4 months ago

Unfortunately, this leads to other problems. I just got an "Argument string may not be null" error, which forced me to clear the history to be able to restart the extension. To reproduce :

Take a screenshot to obtain an image in the clipboard.
Bring up the panel and click on the favorites star in the search bar.
Click again on the favorites star in the search bar.
The image has disappeared from the panel and restarting a session gives the error message.

If it helps. That's all for today!

Totto16 commented 4 months ago

Unfortunately, this leads to other problems. I just got an "Argument string may not be null" error, which forced me to clear the history to be able to restart the extension. To reproduce :
1. Take a screenshot to obtain an image in the clipboard.

2. Bring up the panel and click on the favorites star in the search bar.

3. Click again on the favorites star in the search bar.

4. The image has disappeared from the panel and restarting a session gives the error message.
If it helps. That's all for today!

Yes that helps ❤️ I'll look into it, I think i might know the cause of that issues 👍🏼

oae / gnome-shell-pano

Repeated backslash escape #196

Description

Problem Explanation

Expected Behavior

Reprodution

Steps To Reproduce

Details

It happens with any application?

It happens only on one computer?

It happens only with some specific gnome configuration?

It happens only with some specific extension installed?

Diagnostics

Environment

Packages required

Method

Remarks

Warning about clearing history