Skip to content

Commit

Permalink
Follow-up #12035 and #12068: Show an informative progress indicator w…
Browse files Browse the repository at this point in the history 
…hen dragging and dropping PDF files to JabRef (#12072)

* Updated changes following up for pull requests #12035 and #12068

* Updated CHANGELOG.md to display correct pull request

* Updated StringUtil.shortenFileName to handle maxLength null case and added unit tests for maxLength null cases

* Updated CHANGELOG.md to follow format and be more concise

* Moved shortenFileName() to FileUtil

* Refactored FileUtil.shortenFileName to better use of maxLength and Updated progress messages for Importing, Indexing files.

---------

Co-authored-by: Baron Mac <[email protected]>
  • Loading branch information
@BaronMac08 @mlduyy
BaronMac08 and mlduyy authored Oct 26, 2024
1 parent 08aa10c commit 61d342a
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We added automatic browser extension install on Windows for Chrome and Edge. [#6076](https://github.com/JabRef/jabref/issues/6076)
- We added a search bar for filtering keyboard shortcuts. [#11686](https://github.com/JabRef/jabref/issues/11686)
- By double clicking on a local citation in the Citation Relations Tab you can now jump the linked entry. [#11955](https://github.com/JabRef/jabref/pull/11955)
- We use the menu icon for background tasks as a progress indicator to visualise an import's progress when dragging and dropping several PDF files into the main table. [#12072](https://github.com/JabRef/jabref/pull/12072)

### Changed

Expand Down
18 changes: 12 additions & 6 deletions src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public BackgroundTask<List<ImportFilesResultItemViewModel>> importFilesInBackgro
return new BackgroundTask<>() {
private int counter;
private final List<ImportFilesResultItemViewModel> results = new ArrayList<>();
private final List<BibEntry> allEntriesToAdd = new ArrayList<>();

@Override
public List<ImportFilesResultItemViewModel> call() {
Expand All @@ -115,8 +116,13 @@ public List<ImportFilesResultItemViewModel> call() {
}

UiTaskExecutor.runInJavaFXThread(() -> {
updateMessage(Localization.lang("Processing file %0", file.getFileName()));
updateProgress(counter, files.size() - 1d);
setTitle(Localization.lang("Importing files into %1 | %2 of %0 file(s) processed.",
files.size(),
bibDatabaseContext.getDatabasePath().map(path -> path.getFileName().toString()).orElse(Localization.lang("untitled")),
counter));
updateMessage(Localization.lang("Processing %0", FileUtil.shortenFileName(file.getFileName().toString(), 68)));
updateProgress(counter, files.size());
showToUser(true);
});

try {
Expand Down Expand Up @@ -168,10 +174,7 @@ public List<ImportFilesResultItemViewModel> call() {

UiTaskExecutor.runInJavaFXThread(() -> updateMessage(Localization.lang("Error")));
}

// We need to run the actual import on the FX Thread, otherwise we will get some deadlocks with the UIThreadList
// That method does a clone() on each entry
UiTaskExecutor.runInJavaFXThread(() -> importEntries(entriesToAdd));
allEntriesToAdd.addAll(entriesToAdd);

ce.addEdit(new UndoableInsertEntries(bibDatabaseContext.getDatabase(), entriesToAdd));
ce.end();
Expand All @@ -180,6 +183,9 @@ public List<ImportFilesResultItemViewModel> call() {

counter++;
}
// We need to run the actual import on the FX Thread, otherwise we will get some deadlocks with the UIThreadList
// That method does a clone() on each entry
UiTaskExecutor.runInJavaFXThread(() -> importEntries(allEntriesToAdd));
return results;
}

Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/jabref/logic/search/indexing/DefaultLinkedFilesIndexer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.jabref.logic.util.BackgroundTask;
import org.jabref.logic.util.HeadlessExecutorService;
import org.jabref.logic.util.StandardFileType;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
Expand Down Expand Up @@ -133,8 +134,6 @@ private void addToIndex(Map<String, Pair<Long, Path>> linkedFiles, BackgroundTas
return;
}

task.setTitle(Localization.lang("Indexing PDF files for %0", libraryName));
task.showToUser(true);
LOGGER.debug("Adding {} files to index", linkedFiles.size());
int i = 1;
for (Map.Entry<String, Pair<Long, Path>> entry : linkedFiles.entrySet()) {
Expand All @@ -143,8 +142,10 @@ private void addToIndex(Map<String, Pair<Long, Path>> linkedFiles, BackgroundTas
return;
}
addToIndex(entry.getKey(), entry.getValue().getKey(), entry.getValue().getValue());
task.setTitle(Localization.lang("Indexing files for %1 | %2 of %0 file(s) indexed.", linkedFiles.size(), libraryName, i));
task.updateProgress(i, linkedFiles.size());
task.updateMessage(Localization.lang("Indexing %0. %1 of %2 files added to the index.", entry.getValue().getValue().getFileName(), i, linkedFiles.size()));
task.updateMessage(Localization.lang("Indexing %0", FileUtil.shortenFileName(entry.getValue().getValue().getFileName().toString(), 68)));
task.showToUser(true);
i++;
}
LOGGER.debug("Added {} files to index", linkedFiles.size());
Expand Down
62 changes: 62 additions & 0 deletions src/main/java/org/jabref/logic/util/io/FileUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public class FileUtil {
public static final int MAXIMUM_FILE_NAME_LENGTH = 255;

private static final Logger LOGGER = LoggerFactory.getLogger(FileUtil.class);
private static final String ELLIPSIS = "...";
private static final int ELLIPSIS_LENGTH = ELLIPSIS.length();

/**
* MUST ALWAYS BE A SORTED ARRAY because it is used in a binary search
Expand Down Expand Up @@ -523,6 +525,66 @@ public static boolean detectBadFileName(String fileName) {
return false;
}

/**
* Shorten a given file name in the middle of the name using ellipsis. Example: verylongfilenameisthis.pdf
* with maxLength = 20 is shortened into verylo...isthis.pdf
*
* @param fileName the given file name to be shortened
* @param maxLength the maximum number of characters in the string after shortening (including the extension)
* @return the original fileName if fileName.length() <= maxLength. Otherwise, a shortened fileName
*/
public static String shortenFileName(String fileName, Integer maxLength) {
if (fileName == null || maxLength == null || maxLength < ELLIPSIS_LENGTH) {
return "";
}

if (fileName.length() <= maxLength) {
return fileName;
}

String name;
String extension;

extension = FileUtil.getFileExtension(fileName).map(fileExtension -> '.' + fileExtension).orElse("");
if (extension.isEmpty()) {
name = fileName;
} else {
name = fileName.substring(0, fileName.length() - extension.length());
}

int totalNeededLength = ELLIPSIS_LENGTH + extension.length();
if (maxLength <= totalNeededLength) {
return fileName.substring(0, maxLength - ELLIPSIS_LENGTH) + ELLIPSIS;
}

int charsForName = maxLength - totalNeededLength;
if (charsForName <= 0) {
return ELLIPSIS + extension;
}

int numCharsBeforeEllipsis;
int numCharsAfterEllipsis;
if (charsForName == 1) {
numCharsBeforeEllipsis = 1;
numCharsAfterEllipsis = 0;
} else {
// Allow the front part to have the extra in odd cases
numCharsBeforeEllipsis = (charsForName + 1) / 2;
numCharsAfterEllipsis = charsForName / 2;
}

numCharsBeforeEllipsis = Math.min(numCharsBeforeEllipsis, name.length());
numCharsAfterEllipsis = Math.min(numCharsAfterEllipsis, name.length() - numCharsBeforeEllipsis);

StringBuilder result = new StringBuilder();
result.append(name, 0, numCharsBeforeEllipsis)
.append(ELLIPSIS)
.append(name.substring(name.length() - numCharsAfterEllipsis))
.append(extension);

return result.toString();
}

public static boolean isCharLegal(char c) {
return Arrays.binarySearch(ILLEGAL_CHARS, c) < 0;
}
Expand Down
7 changes: 4 additions & 3 deletions src/main/resources/l10n/JabRef_en.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ Extract\ References\ (online)=Extract References (online)
Processing...=Processing...
Processing\ "%0"...=Processing "%0"...
Processing\ PDF(s)=Processing PDF(s)
Processing\ file\ %0=Processing file %0
Processing\ %0=Processing %0
Importing\ files\ into\ %1\ |\ %2\ of\ %0\ file(s)\ processed.=Importing files into %1 | %2 of %0 file(s) processed.
Processing\ a\ large\ number\ of\ files=Processing a large number of files

You\ are\ about\ to\ process\ %0\ files.\ Continue?=You are about to process %0 files. Continue?
Expand Down Expand Up @@ -490,10 +491,10 @@ Independent\ group\:\ When\ selected,\ view\ only\ this\ group's\ entries=Indepe
I\ Agree=I Agree

Indexing\ bib\ fields\ for\ %0=Indexing bib fields for %0
Indexing\ PDF\ files\ for\ %0=Indexing PDF files for %0
Indexing\ %0=Indexing %0
Indexing\ files\ for\ %1\ |\ %2\ of\ %0\ file(s)\ indexed.=Indexing files for %1 | %2 of %0 file(s) indexed.
%0\ of\ %1\ entries\ added\ to\ the\ index.=%0 of %1 entries added to the index.
%0\ of\ %1\ entries\ removed\ from\ the\ index.=%0 of %1 entries removed from the index.
Indexing\ %0.\ %1\ of\ %2\ files\ added\ to\ the\ index.=Indexing %0. %1 of %2 files added to the index.
Removing\ entries\ from\ index\ for\ %0=Removing entries from index for %0
Invalid\ URL=Invalid URL

Expand Down
25 changes: 25 additions & 0 deletions src/test/java/org/jabref/logic/util/io/FileUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

Expand Down Expand Up @@ -477,4 +478,28 @@ void legalPaths(String fileName) {
void illegalPaths(String fileName) {
assertTrue(FileUtil.detectBadFileName(fileName));
}

@ParameterizedTest
@CsvSource({
"'' , , ",
"'' , , -3",
"'' , , 0",
"'' , , 3",
"'' , , 5",
"'' , , 10",
"'' , thisisatestfile.pdf , ",
"'' , thisisatestfile.pdf , -5",
"'' , thisisatestfile.pdf , 0",
"... , thisisatestfile.pdf , 3",
"th... , thisisatestfile.pdf , 5",
"th...e.pdf , thisisatestfile.pdf , 10",
"thisisatestfile.pdf , thisisatestfile.pdf , 20",
"lo... , longfilename.extremelylongextension , 5",
"longfil... , longfilename.extremelylongextension , 10",
"longfilename.extr... , longfilename.extremelylongextension , 20",
"lo...me.extremelylongextension , longfilename.extremelylongextension , 30",
})
void shortenFileName(String expected, String fileName, Integer maxLength) {
assertEquals(expected, FileUtil.shortenFileName(fileName, maxLength));
}
}

0 comments on commit 61d342a

Please sign in to comment.