tsujan / FeatherPad

Lightweight Qt Plain-Text Editor for Linux
GNU General Public License v3.0
390 stars 66 forks source link

Double click a file opens a blank page in Haiku. #730

Closed khallebal closed 1 year ago

khallebal commented 1 year ago

Hi @tsujan , as the title says it already, but also using openwith->featherpad from the rightclick menu opens a blank page instead of the selected file. V1.1.0 still works normally though, using the terminal with the filename as an argument also works, any hints on how to fix this or what has changed since since V1.1.0?, i am unable to debug it using terminal coz as i mentioned it works when using terminal. i hope you'll have some insight on what's causing this behavior. Cheers.

tsujan commented 1 year ago

A file can be (double) clicked only inside a file manager (FM), and that FM controls what happens. In your case, for some reason, the FM doesn't give the file path to FeatherPad. I have no idea why.

what has changed since since V1.1.0?

Many things, but I don't think any of them is relevant. However, I'll take a look at the differences between two core files later.

Did you compile FeatherPad against Qt5 or Qt6?

tsujan commented 1 year ago

I checked the code. There's nothing in the code that makes a distinction between opening in terminal and opening by (double) clicking in a FM. Therefore, it can't be about the code.

There remains one possibility, namely, CMakeLists.txt. I haven't changed anything related to Haiku in it. Are you sure https://github.com/tsujan/FeatherPad/pull/728 isn't the cause?

khallebal commented 1 year ago

Did you compile FeatherPad against Qt5 or Qt6?

Tried with both.

Are you sure https://github.com/tsujan/FeatherPad/pull/728 isn't the cause?

I disabled the socketpair related code and the change from #728 still it doesn't work.

I will try to build my way up from v1.1.0 and narrow it down to where it started hopefully we'll get a better idea what broke it, i'll keep you posted.

tsujan commented 1 year ago

I will try to build my way up from v1.1.0

That could be tedious and might not provide a reliable info.

The main difference of FeatherPad ≥ 1.3.1 from the previous versions is D-Bus. It relies on QDBusConnection::isConnected(): if it returns false — as it should in an OS without D-Bus (which was tested in macOS — not by me) — D-Bus won't be used.

Now, I don't know how Haiku treats D-Bus. But the strange thing is that you say it works in a terminal. These questions come to my mind:

khallebal commented 1 year ago

Does Haiku's FM behave differently when it comes to D-Bus?

Haiku's FM does not make use of it at all, and i don't have it installed. I will install it and see if it makes a difference. Is QD-Bus the same as the original D-Bus?

Can another FM be used under Haiku?

I have a Qt FM that i can try, maybe it really needs D-Bus now?

The main difference of FeatherPad ≥ 1.3.1 from the previous versions is D-Bus

So this change is not part FeatherPad-1.3.0?. I've already tried with V1.3.5 ,it's the same result.

tsujan commented 1 year ago

Haiku's FM does not make use of it at all, and i don't have it installed.

Which FM do you use? Is it specific to Haiku, or can it be installed under Linux too?

Is QD-Bus the same as the original D-Bus?

No. the former just uses the latter if available. Qt doesn't depend on D-Bus.

So this change is not part FeatherPad-1.3.0?.

No. Using of D-Bus started from 1.3.1 (see https://github.com/tsujan/FeatherPad/releases).

Do you see the problem with 1.3.0 too?

khallebal commented 1 year ago

Which FM do you use? Is it specific to Haiku, or can it be installed under Linux too?

It's specific to Haiku, integrated to Haiku's DE, i tried porting PcmanFM_qt but it was difficult, it needs haiku specific code for opening files (haiku does not use .desktop files for that, it uses a different API). If you have the time and motivation to help me port it, it'll be great; i have already a patch that gets it compiled and run but as i said, it can't open files but does open folders.

Do you see the problem with 1.3.0 too?

I haven't tried it yet, i'm gonna do it now; i'll let you know.

tsujan commented 1 year ago

i tried porting PcmanFM_qt but it was difficult,

Unlike FeatherPad, it doesn't work without D-Bus.

i'm gonna do it now; i'll let you know.

Waiting to know the result....

khallebal commented 1 year ago

Waiting to know the result....

Okay, I installed D-Bus_devel and built featherpad-1.4.0, it didn't help, then i tried opening files using a Qt based FM aaand... it worked and then i built featherpad-1.3.0, this time it worked with Haiku's FM, so it might no bet a D-Bus issue after all? could it be the new "signalDaemon" API that's interfering with the opening of files?

tsujan commented 1 year ago

i built featherpad-1.3.0, this time it worked with Haiku's FM,

1.3.0 is the last version without D-Bus. So, most probably (although not 100%), the problem may be related to D-Bus.

could it be the new "signalDaemon" API that's interfering with the opening of files?

No. It's about quitting with Unix signals, not file openeing. Moreover, the most interesting observation of yours was that it worked in a terminal.

tsujan commented 1 year ago

i tried opening files using a Qt based FM aaand... it worked

Oh, this shows that Haiku's FM is confused somehow. Why it wasn't confused by versions that didn't use D-Bus, I have no idea yet. We need to solve this Haiku mystery.

khallebal commented 1 year ago

We need to solve this Haiku mystery.

I took a closer look to the D-Bus implementation and i have to say i don't see anything suspicious, it should just work with systems like Haiku which don't use D-Bus?....it's a mystery indeed... Any other suggestion?

tsujan commented 1 year ago

Any other suggestion?

I have 3 reasons that the problem is outside FeatherPad:

  1. There isn't (and can't be) anything in the code about how the arguments (char **argv in main()) are provided;
  2. It works in a terminal (your test); and
  3. It works with Qt-based FMs (your test again).

The mystery is why Haiku's FM had no problem with it before D-Bus was introduced.

khallebal commented 1 year ago

The mystery is why Haiku's FM had no problem with it before D-Bus was introduced.

That's a good question?, is there a way i can disable it temporarely just to see what happens? it seems a bit nested.

tsujan commented 1 year ago

is there a way i can disable it temporarely

If you don't have D-Bus, there's nothing to disable.

For macOS, Qt doc says, "... applications linking against the QtDBus module will load even on macOS systems that do not have the libraries, but they will fail to connect to any D-Bus server and they will fail to open a server using QDBusServer."

This is the only reasonable approach everywhere: fail to connect without D-Bus. Apparently, that's the case for OS/2 too. But perhaps it isn't true for Haiku.

You could test this hypothesis by applying a patch like this:

diff -ruNp featherpad-orig/featherpad/singleton.cpp featherpad/featherpad/singleton.cpp
--- featherpad-orig/featherpad/singleton.cpp
+++ featherpad/featherpad/singleton.cpp
@@ -65,6 +65,10 @@ FPsingleton::FPsingleton (int &argc, cha
     isPrimaryInstance_ = standalone_;
     if (!standalone_)
     {
+#if defined(Q_OS_HAIKU)
+        isPrimaryInstance_ = true;
+        standalone_ = true;
+#else
         QDBusConnection dbus = QDBusConnection::sessionBus();
         if (!dbus.isConnected()) // interpret it as the lack of D-Bus
         {
@@ -77,6 +81,7 @@ FPsingleton::FPsingleton (int &argc, cha
             new FeatherPadAdaptor (this);
             dbus.registerObject (QStringLiteral ("/Application"), this);
         }
+#endif
     }
 }
 /*************************/

If it doesn't work after that, the mystery will be deepened and narrowed down at the same time. I think it's a good test. Please tell me the result.

khallebal commented 1 year ago

Still the same, works with Qt based FM but not with Haiku's FM, any other new code that may seem like a good candidate?

tsujan commented 1 year ago

Still the same

So, the problem may not be about D-Bus after all. Strange!

khallebal commented 1 year ago

Could it a possible that the problem stems from the logic in main.cpp of how arguments are presented? the following patch fixed the issue in haiku, i don't if it's the correct way (i'm not a dev), but it works.


index 044f5a6f..772f1f93 100644
--- a/featherpad/main.cpp
+++ b/featherpad/main.cpp
@@ -120,10 +120,10 @@ int main (int argc, char **argv)
     int d = -1;
 #endif
     info << QString::number (d) << QDir::currentPath();
-    if (argc > 1)
+    if (argc > 0)
     {
         info << firstArg;
-        for (int i = 2; i < argc; ++i)
+        for (int i = 1; i < argc; ++i)
             info << QString::fromUtf8 (argv[i]);
     }
`
tsujan commented 1 year ago

the following patch fixed the issue in haiku,

firstArg is already equal to QString::fromUtf8 (argv[1]). With the above change, you duplicate it and open a file twice (that happens here)

argc is 1 when the app is opened without an argument; then argv[0] is the name/path of the program's executable. if you pass one argument — e.g. a file path — then argc is 2.

I think you've found a bug in Haiku's FM.

khallebal commented 1 year ago

Sorry to bother you again @tsujan; but i thought you may want to see this and explain to me what the difference is between FeatherPad and FeatherNotes in this case. The following patch is taken from the FeatherNotes's implimentation which is identical to FeatherPad's one except for the part shown in the patch, the reason i tried this, is because i noticed that FeatherNotes worked and FeatherPad didn't, and now FeatherPads works with this too, the only thing that's different from the V1.3.0 is that now opening a second file gets opened in a new window instead of new tab.


index 044f5a6f..4222a28f 100644
--- a/featherpad/main.cpp
+++ b/featherpad/main.cpp
@@ -122,9 +122,9 @@ int main (int argc, char **argv)
     info << QString::number (d) << QDir::currentPath();
     if (argc > 1)
     {
-        info << firstArg;
-        for (int i = 2; i < argc; ++i)
-            info << QString::fromUtf8 (argv[i]);
+        info << QString::fromUtf8 (argv[1]);
+        if (argc > 2)
+            info << QString::fromUtf8 (argv[2]);
     }
`
tsujan commented 1 year ago

Logically they should make no difference, as they don't in Linux and macOS — they don't under Haiku and with a terminal emulator or a Qt FM either, as you tested. Why they make a difference with Haiku's FM, I have no idea!

khallebal commented 1 year ago

I can make a patch and keep it locally at Haikuports it's no big deal, but do you an idea how to fix the other issue which is opening a second file gets opened in a new window instead of a new tab as it used to previously behave?

tsujan commented 1 year ago

Please take a look at https://discuss.haiku-os.org/t/open-with-in-qt5-application/9132?page=2

It's about a similar issue and tells something about Q_REF_TO_ARGV and more.

khallebal commented 1 year ago

Please take a look at https://discuss.haiku-os.org/t/open-with-in-qt5-application/9132?page=2

Yes i've seen it, the workaround suggested there is add a flag to the binary, which i did and worked up until V1.3.0 then it didn't anymore. I'll take another look and see if there is something else to be done.

tsujan commented 1 year ago

My main question is this: Why does https://github.com/tsujan/FeatherPad/issues/730#issuecomment-1536376217 work with Haiku's FM?! It should make no difference whatsoever.

IMHO, this question should be answered first.

tsujan commented 1 year ago

A weird explanation comes to my mind, but it's the only one that I've been able to think of until now:

For some reason, Haiku's FM does not provide the arguments as other apps do. But Qt changes that in QApplication::QApplication(int &argc, char **argv) (this function may change argc and argv, as Qt doc says), so that, after it is called, argc and argv become standard.

I'll change the code, so that argc and argv aren't used before calling the c-tor of QApplication. Then I might need to use QCommandLineParser instead of the current code — a lot of work.

khallebal commented 1 year ago

For some reason, Haiku's FM does not provide the arguments as other apps do.

Before you make any changes, you may want to take a look at the be/haiku API about that to get a better understanding, it doesn't mention anything special that might seem suspicious AFAICT.

tsujan commented 1 year ago

Yes, I'd found similar docs; nothing suspicious. However, we are seeing a very suspicious behavior here ;) Although it happens only with Haiku's FM, it's better be safe than sorry.

On the other hand, it's a long time I want to use QCommandLineParser instead of adding options manually but have delayed it. This is a good reason to try it at least.

khallebal commented 1 year ago

On the other hand, it's a long time I want to use QCommandLineParser

Alright then, i did know that, hopefully it will fix it once and for all, i'll wait for it before updating the haikuports recipe, or if V1.4.1 comes out first i will use the above mentioned patch, thanks so much for your assistance tsujan.

tsujan commented 1 year ago

Before dealing with QCommandLineParser, I need to make sure that my theory is correct. Therefore, I'll make a small commit and ask you for a test. If it works with Haiku's FM, the mystery will be solved.

Will tell you when it's ready.

tsujan commented 1 year ago

@khallebal Please test the latest commit https://github.com/tsujan/FeatherPad/commit/60fda551f78b0c2038aae455895e853e8388ac03

khallebal commented 1 year ago

It does open files by double clicking and by using the OpenwWith menu, though the behavior when opening a second file has not change (e.i, a new window is opened), this wasn't the case before V1.3.1, i thought this and that were related, but it seems it's not the case, but this one is less urgent, when ever you have time to think about it......, in the mean time i'll try to figure out a solution for it myself.

tsujan commented 1 year ago

e.i, a new window is opened), this wasn't the case before V1.3.1

That's not a bug. The reason is that, now, D-Bus is needed for having a single-instance app, but Haiku doesn't have D-Bus. Please read the first section of the Help doc for a short explanation. The reason for switching to D-Bus was that it's much more efficient and reliable that the older method.

Without D-Bus, you could still open multiple files in the same window by opening them simultaneously. Also, file drag-and-drop is possible, although I don't know whether Haiku supports it.

Anyway, mystery solved :) Tthe whole story was about non-UTF8 arguments in Haiku :

Haiku's FM doesn't use UTF8, and because of that, QString::fromUtf8() doesn't work with argv directly. But once QCoreApplication is called, it fixes the situation by changing argv to UTF8. That's why there was no problem on Linux or with a Qt-based FM on Haiku. In the patch, I just let Qt do its job first, without assuming that a UTF8 string is received (which is always the case on Linux).

I'll make similar changes to my other apps for the sake of certainty.

Thanks a lot for the discussion and also for the test!

khallebal commented 1 year ago

Also, file drag-and-drop is possible, although I don't know whether Haiku supports it.

Haiku does have support for D&D, and works FeatherPad.

Haiku's FM doesn't use UTF8

Are you sure about this? that is quite surprising to hear!, if it were another encoding i would believe it right away, i'll ask around some devs and see what they know about it, i'm probably the only one who doesn't know this.

tsujan commented 1 year ago

Are you sure about this?

~100%. It remains no doubt after your observations and my change.~

EDIT: To my big surprise, it isn't the case for Haiku; read @nephele-gh's explanations below and also https://github.com/tsujan/FeatherPad/issues/730#issuecomment-1537142622

that is quite surprising to hear!

It's not a sin ;) But it's like closing the door to the rest of the world.

I think their code is old. I remember I saw something similar in a Unix flavor (I don't remember where).

ghost commented 1 year ago

Hi, Haiku used only utf-8, and nothing else. So this seems like a wierd observation.

Normally launching applications via tracker does not send an argv at all, instead it is delivered as a message to the application on which file to open.

Now, I don't know how the qt specific flags mentioned work, but iirc not using them and implementing QFileOpenEvent should make this work properly.

Here is the code in qthaikuplugins that sends this: https://github.com/threedeyes/qthaikuplugins/blob/98be82eb11349358e35bfa653edddd915237de7f/platforms/qhaikuplatform/haiku/qhaikuapplication.cpp#L133

I think their code is old. I remember I saw something similar in a Unix flavor (I don't remember where).

Haiku is about 20, so relatively young for an OS ;)

ghost commented 1 year ago

That's not a bug. The reason is that, now, D-Bus is needed for having a single-instance app, but Haiku doesn't have D-Bus.

Haiku supports this natively by setting the single launch flag on the binary. (B_SINGLE_LAUNCH)

What do you need this for, I wonder though. ;)

dbus does work on haiku but it is fairly uneccesary as there is already a message passing system in place, also it makes my shutdown slower :)

tsujan commented 1 year ago

Now, I don't know how the qt specific flags mentioned work,

This wasn't a Qt issue. The argument that Haiku's FM provided wasn't in UTF8. Qt corrected this.

ghost commented 1 year ago

Haiku only uses utf-8, so unless this is an issue specific to qthaikuplugins this seems very unlikely.

Your from utf8 function not working before the QApplication exists is likely caused because a QApplication is required to actually deliver the arguments, because it is not passed as argv by Haiku at all and instead by a BMessage which is then translated to either the native QFileOpenEvent or to the argv qt api if you set the right workaround flags. You can read the code in QApplication from qthaikuplugins above for this, I don't see anything there that would fix a wrong encoding either.

tsujan commented 1 year ago

Your from utf8 function not working before the QApplication exists is likely caused because a QApplication is required to actually deliver the arguments

No, QString::fromUtf8() has no relation to QApplication. Getting the first argument immediately worked everywhere except for Haiku's FM — it even worked in Haiku but outside its FM.

ghost commented 1 year ago

nothing is passed as argv, at all. the QApplication is absolutely required to use the workaround you wish to employ to fill the argv from the info passed as a BMessage to the QApplication. Without it this will not work and the argv will remain empty. The right solution is implementing QFileOpenEvent.

This has nothing to do with tracker, or open, but the native way apps are asked to open files on Haiku, by passing a message and not starting the application again with a filepath argument in their argv. This only worked in Terminal because the argv was filled in by the user like you expected before the QApplication was created.

tsujan commented 1 year ago

@nephele-gh

It seems that you suppose that I know about Haiku. I simply don't. FeatherPad's Haiku support isn't mine.

What I know is that, having a line like

const QString s = QString::fromUtf8(argv[1]);

at the very start of the main function — before calling QApplication — and giving it to the app works everywhere except for Haiku's FM. The logical conclusion is that argv[1] is not in UTF8 in the case of Haiku's FM. How that argument is created when Haiku's FM is used shouldn't be relevant to the app.

ghost commented 1 year ago

No, it does not work because it is empty. It has nothing to do with encodings.

tsujan commented 1 year ago

Why is it empty only with Haiku's FM? I ask only out of curiosity, knowing nothing about Haiku's FM.

ghost commented 1 year ago

Haiku passes requests to open a file by sending a message to the already running process via a BMessage. Argv is natively, for gui apps, never used for this purpose.

Terminal is different since the commandline is executed as instructed, but all other invocations will pass the to be opened file as a message.

In the case of the qt port ported applications sometimes expect the arg to be in argv, but this only works because the message for the file is accepted by QApplication if a flag is set and then filled into the argv, so this workaround fails if you try to read it before QApplication is created.

tsujan commented 1 year ago

Thanks! Although I don't know how Haiku's BMessage works, I can see the general picture. In short, you're saying that this is how Qt can work in Haiku.

Is the same true about GTK and other toolkits?

ghost commented 1 year ago

You would need similar workarounds yes.

In case of qt there is a native way to support this though, qt native that is: https://doc.qt.io/qt-5/qfileopenevent.html

With this api you can support opening files later in the process lifecycle too, and the argv workaround becomes uneccesary.

tsujan commented 1 year ago

Now everything is clear. Thanks a lot for your explanations!

My mistake was that I thought the lack of argv was impossible; having ruled out impossible things, I concluded that the problem should be about non-UTF8 strings.

Not handling argv before calling QCoreApplication is also good when the possibility of non-UTF8 strings exists.

tsujan commented 1 year ago

Mentioned it in a ChangeLog entry: https://github.com/tsujan/FeatherPad/commit/764a33e23fe6cab316095b10e63460b47cd2a70e

khallebal commented 1 year ago

Is the same true about GTK and other toolkits?

Yes gtk apps also exhibit the same behavior, right now i'm going through the gtk docs trying to find a workaround similar to


QT:` QPA_FLAGS”) “QT_REF_TO_ARGV | ``QT_REF_TO_FORK`, So far no luck.