Add ability to use DOI directly in Web Search #9969
Conversation
| @@ -77,6 +84,13 @@ public WebSearchPaneViewModel(PreferencesService preferencesService, DialogServi | |||
| // in case user did not enter something, it is treated as valid (to avoid UI WTFs) | |||
| return null; | |||
| } | |||
|
|
|||
| // check if DOI is present | |||
There was a problem hiding this comment.
This is the code the next line. Please remove this comment.
Add a comment in the line before "null". It should be similar than line 84 "in case user did not enter something, it is treated as valid (to avoid UI WTFs)"
| fetcherName = doiFetcher.getName(); | ||
| } | ||
|
|
||
| String finalFetcherName = fetcherName; |
There was a problem hiding this comment.
This variable is not ncessary - just using fetcherName is enough.
There was a problem hiding this comment.
Thanks for your feedback!
When I try to use fetcherName in the client.trackEvent() call, an error is raised that local variables referenced from a lambda expression must be final or effectively final. Is there a way to pass the value to the lambda without creating the effectively final finalFetcherName variable? A possible solution is to make it an instance variable but I am not sure that's a good idea
There was a problem hiding this comment.
No, then keep it as is. I already though that it's probably used in a lambda
| @@ -130,16 +144,27 @@ public void search() { | |||
| } | |||
|
|
|||
| SearchBasedFetcher activeFetcher = getSelectedFetcher(); | |||
| BackgroundTask<ParserResult> task; | |||
There was a problem hiding this comment.
This variable can be introuced in line 162, can't it? In JabRef, we introduce the variable at the latest point possible - not somewhere inbetween.
koppor
added
the
status: ready-for-review
| .withInitialMessage(Localization.lang("Processing %0", query)); | ||
| task.onFailure(dialogService::showErrorDialogAndWait); | ||
|
|
||
| ImportEntriesDialog dialog = new ImportEntriesDialog(stateManager.getActiveDatabase().get(), task); | ||
| dialog.setTitle(activeFetcher.getName()); | ||
| dialog.setTitle(fetcherName); |
There was a problem hiding this comment.
The "new" variable should also be used - otherwise, it is confusing IMHO.
| dialog.setTitle(fetcherName); | |
| dialog.setTitle(finalFetcherName); |
| fetcherName = doiFetcher.getName(); | ||
| } | ||
|
|
||
| String finalFetcherName = fetcherName; |
There was a problem hiding this comment.
Then I would also state that in the code:
| String finalFetcherName = fetcherName; | |
| final String finalFetcherName = fetcherName; |
There was a problem hiding this comment.
Got it, I've updated the PR with these changes
This fixes #9674 by checking whether the query string contains a valid DOI. If yes, the
DOIFetcheris used instead of the selected fetcher. This is also reflected in theImportEntriesDialogtitle.Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)