Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working by XDZhelheim · Pull Request #7551 · JabRef/jabref

XDZhelheim · 2021-03-18T12:54:04Z

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Video:

fix6487.mp4

Use D:\TEST\test whitespace.bib as an example.

Double-click it, and I got errors in console as the following:

Error opening file 'D:\TEST\test': Could not find a suitable import format.
Error opening file 'whitespace.bib': Could not find a suitable import format.
21:41:53.032 [JabRef - Remote Listener Server on port 6050] ERROR org.jabref.logic.importer.OpenDatabase - Error: File not found

Therefore, the path was regarded as two files: D:\TEST\test and whitespace.bib.

Start from org.jabref.logic.importer.OpenDatabase to backtrack the dataflow:

Class	Method	Variable
`OpenDatabase`	`loadDatabase`	`fileToOpen`
`ArgumentProcessor`	`importAndOpenFiles`	`aLeftOver`
`JabRefCLI`	constructor	`args`
`ArgumentProcessor`	constructor	`args`
`JabRefMessageHandler`	`handleCommandlineArguments`	`message`
`RemoteListenerServer`	`handleMessage`	`argument`
`RemoteListenerServer`	`run`	`input.getValue()`
`Protocol`	`receiveMessage`	`argument`

Use logger to print the variable argument:

if (argument instanceof String[]) {
    LOGGER.info(Arrays.toString((String[]) argument));
}

And I got:

21:41:53.024 [JabRef - Remote Listener Server on port 6050] INFO  org.jabref.logic.remote.shared.Protocol - [D:\TEST\test, whitespace.bib]

So I think the problem is that when passing the argument D:\TEST\test whitespace.bib, it will be considered as two arguments because there is a whitespace that serves as a delimiter.

I noticed that @zongxian said that a path containing Chinese characters would cause the same error. So the problem is not just related to delimiter, probably about text encoding.

Therefore I encoded the arguments in RemoteClient.sendCommandLineArguments and decoded them in Protocol.receiveMessage.

Then I created a binary and tested. Now it works well on my computer (Windows 10 1909).

update 2021.3.3

update 2021.3.8

update 2021.3.9

update 2021.3.11

update 2021.3.12

update 2021.3.15

update 2021.3.17

who invoked the method sendMessage(type=SEND_COMMAND_LINE_ARGUMENTS) when double-clicking

tobiasdiez

Thanks a lot for your detailed analysis! The fix looks good to me.

The linked issue also mentions that double clicking a file without having jabref open doesn't work sometimes (if I got the gist right by skimming it). I guess this is not fixed with this PR, right?

tobiasdiez · 2021-03-18T14:26:34Z

        try (Protocol protocol = openNewConnection()) {
-            protocol.sendMessage(RemoteMessage.SEND_COMMAND_LINE_ARGUMENTS, args);
+
+            String[] encodedArgs = args.clone();


Can you please put this addition in the protocol.sendMessage so that only the protocol has to worry about decoding/encoding. Also please add a short comment explaining why we need to encode. Thanks!

created a new test in RemoteCommunicationTest

XDZhelheim · 2021-03-19T03:40:01Z

Thanks for your acknowledgement!

I moved it to Protocol.sendMessage and added some comments.

Besides, I created a new test case in RemoteCommunicationTest.

As @RickVandoorne mentioned:

If I double click on a .bib file while JabRef is NOT running then JabRef opens (it no longer gives me the error message). However, it does not open the file which I double clicked.

If I double click on a .bib file while JabRef is running it opens a new library called "untitled" but not the file I double clicked.

If I open JabRef first and then open a file using "File" → "Open library" it works fine.

I tested these three ways to open a .bib file, got no error.

Double click while JabRef not running:
https://user-images.githubusercontent.com/57553691/111728161-47c5e680-88a7-11eb-81d2-ddd56253b3da.mp4

Double click while JabRef running:
Video above.

"Open library":

open.library.mp4

XDZhelheim · 2021-03-19T03:50:50Z

+        client.sendCommandLineArguments(message.clone());

        verify(server).handleCommandLineArguments(message);
    }


I am wondering whether this test case is appropriate. Not so sure about how to write a test about socket communication.

Yeah, it's hard to test this. I think, your added test is fine.

tobiasdiez · 2021-03-19T09:52:16Z

        final String[] message = new String[]{"my message"};

-        client.sendCommandLineArguments(message);
+        client.sendCommandLineArguments(message.clone());


I would move the clone call to the sendMessage method. It's usually a good practice to not modify parameters passed to a method.

Done. Thank you for your advice!

Siedlerchr

thanks for the fix! LGTM as well

tobiasdiez

Thanks again for your contribution! Looking forward to your next PR 🚀

* upstream/master: (191 commits) Fix for issue 7416: font size of the preferences dialog does not update with the rest of the GUI. (#7509) Fix school/instituation is printed twice (#7574) Dsiable notarisation until we hae an account for JabRef e.V. (#7572) Fix citation keys unintentionally being overwritten on import (#7443) Fix AuthentificationPlugin not declared in mergedModule (#7570) Suggestions for changes in caching latex free authors (#7301) Add simple Unit Tests (#7542) Fix drag and drop into empty library (#7555) Bump richtextfx from 0.10.4 to 0.10.6 (#7563) Bump pdfbox from 2.0.22 to 2.0.23 (#7561) Bump org.eclipse.jgit (#7560) Bump fontbox from 2.0.22 to 2.0.23 (#7562) Bump guava from 30.1-jre to 30.1.1-jre (#7564) Bump xmpbox from 2.0.22 to 2.0.23 (#7565) Bump hmarr/auto-approve-action from v2.0.0 to v2.1.0 (#7566) Add gource (#7193) UI: Fix for group icon (#7552) Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working (#7551) add ability to insert arxivId (#7549) Fixed missing trigger for linked file operations (#7548) ...

XDZhelheim and others added 12 commits March 3, 2021 10:27

Merge pull request #1 from JabRef/master

4851e84

update 2021.3.3

Merge pull request #2 from JabRef/master

ae45307

update 2021.3.8

Merge pull request #3 from JabRef/master

b95ce43

update 2021.3.9

Fix JabRef#7416

fec81da

Merge pull request #5 from JabRef/master

c1434f7

update 2021.3.11

sync with origin

8428b49

Merge pull request #6 from JabRef/master

3fd7a0d

update 2021.3.12

Merge pull request #7 from JabRef/master

5cc3085

update 2021.3.15

Merge pull request #8 from JabRef/master

d3af958

update 2021.3.17

backtracking file path argument

2f8e151

who invoked the method sendMessage(type=SEND_COMMAND_LINE_ARGUMENTS) when double-clicking

fix JabRef#6487 (draft)

5b723bd

fix

9a5811b

tobiasdiez reviewed Mar 18, 2021

View reviewed changes

moved encoding to sendMessage

fa1885b

created a new test in RemoteCommunicationTest

XDZhelheim commented Mar 19, 2021

View reviewed changes

tobiasdiez reviewed Mar 19, 2021

View reviewed changes

move clone to sendMessage

c9182ee

Siedlerchr approved these changes Mar 19, 2021

View reviewed changes

tobiasdiez approved these changes Mar 19, 2021

View reviewed changes

tobiasdiez merged commit 049acb9 into JabRef:master Mar 19, 2021

sauliusg mentioned this pull request Mar 28, 2021

Allow identical group names #7558

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working#7551

Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working#7551
tobiasdiez merged 14 commits into
JabRef:masterfrom
XDZhelheim:fix-for-issue-6487

XDZhelheim commented Mar 18, 2021 •

edited

Loading

Uh oh!

tobiasdiez left a comment

Uh oh!

tobiasdiez Mar 18, 2021

Uh oh!

XDZhelheim commented Mar 19, 2021

Uh oh!

XDZhelheim Mar 19, 2021

Uh oh!

tobiasdiez Mar 19, 2021

Uh oh!

tobiasdiez Mar 19, 2021

Uh oh!

XDZhelheim Mar 19, 2021

Uh oh!

Siedlerchr left a comment

Uh oh!

tobiasdiez left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

XDZhelheim commented Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

XDZhelheim commented Mar 19, 2021

Uh oh!

XDZhelheim Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

XDZhelheim Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

Siedlerchr left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

XDZhelheim commented Mar 18, 2021 •

edited

Loading