Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Create PDF file Exporter #2706

Closed
wants to merge 10 commits into from
Closed

[WIP] Create PDF file Exporter #2706

wants to merge 10 commits into from

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Apr 2, 2017 •
edited
Loading

Create an exporter that enables the export of Files.
Only caveat: You have to enter an arbitrary filename in the save dialog. The exporter then uses the directory.

Implements #2539

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@Siedlerchr
First approach for implementing PDF Exporter
bd508ad 
Remove code that produces NPE
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into exportPDF
c9eeefd 
* upstream/master:
  fix ID consideration in DuplicateCheck
  Add ArXiv identifier batch lookup (#2710)
  Update mockito from 2.7.19 to 2.7.21
  More defensive identifier list #2708
  Revert "Add more identifier field names #2708"
  Add more identifier field names #2708
  Consider entries as equal if their DOI matches #2708
  Imports
  Imports
  Move duplicate detection to logic
  Reuse edit distance class
  Refactoring
  EntryTypeDialog Fetching Autogenerates BibTeX Key (#2709)
  Add changelog entry
  Increase permitted size of StringUtil
  Make sure that JavaFx shuts down in case another JabRef instance is already open
@Siedlerchr
Export linked files
9ef9760 
Don't replace existings
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 5, 2017
tobiasdiez
tobiasdiez reviewed Apr 5, 2017
View reviewed changes
src/main/java/org/jabref/logic/exporter/IExportFormat.java
* should be exported. The list of entries must be non null
* @throws Exception
*/
void performExport(BibDatabaseContext databaseContext, Path file, Charset encoding, List<BibEntry> entries)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer these export methods that take the file as a Path instead of string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wanted to use it, too. However, the method with that signature never gets called which led to an NPE.
In ExportAction only the one with String is called.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change ExpoortAction then? For now the Path method can be implemented as default and invoking the stringy one.

tobiasdiez
tobiasdiez requested changes Apr 5, 2017
View reviewed changes
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing a test for the new exporter but otherwise it looks good.

Please also add a changelog entry.

src/main/java/org/jabref/logic/exporter/LinkedFileExporter.java Outdated
public class LinkedFileExporter extends ExportFormat {

public LinkedFileExporter() {
super("Files Exporter", "Files", null, null, ".*");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "Export linked files" as name? not sure.

@koppor
Copy link
Member

koppor commented Apr 5, 2017

Should we also move the "Send es email" functionality to the export menu?

@simonharrer
Copy link
Contributor

Hm, I would distinguish between Export and Share. Send as Email sounds to be me like sharing it whereas Export is a lowerlevel primitive which is not always for sharing the data.

@tobiasdiez
Copy link
Member

I like the suggestion by @simonharrer. "Share" fits better than "Export".

@Siedlerchr
Copy link
Member Author

DevCall Decision: Move to Tools -> Copy Files

@tobiasdiez tobiasdiez removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 8, 2017
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into exportPDF
ce4e1ac 
* upstream/master: (39 commits)
  Fix fetcher test
  Allow failures for fetcher test (#2730)
  Use JabRefExecutor service
  Move DOI fetching to separate thread #2682
  Remove gui dependency in logic (#2726)
  Fixed freeze on Mac OS X when creating/editing groups (#2727)
  Only ask once if telemetry data should be collected
  Update wiremock from 2.5.1 to 2.6.0
  Update mockito-core from 2.7.21 to 2.7.22
  Update log4j to latest version
  Azure test (#2724)
  Fix build
  Move expand filename to FileUtil
  Unicode conversion bibtexkey (#2720)
  Add sorting of all groups and subgroups, recursively (#2666)
  Only check capitalization of note and howpublished fields if they start with a word character
  Remove overhauled @author tag
  Implement #1359: collect telemetry (#2283)
  Add licenses of new dependencies
  Fix cssStyleHelper warnings
  ...
@Siedlerchr
Create new Action for exporting files
b035eb4 
TODO: Add to menu
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into exportPDF
01b08d9 
* upstream/master:
  Speedup start by improving Application Insights configuration (#2737)
  Localization: French: Menu: Translation of new entries (#2735)
  Localization: French: General: Translation of new entries (#2736)
  Update Chinese translation. (#2734)
  Fix 2701 too may files found (#2732)
  Update errorprone from 0.0.8 to 0.0.10, because of update of Gradle to 3.5
  Update gradle from 3.4.1 to 3.5
@Siedlerchr
Move to Tools entry
3682e21
@Siedlerchr
Copy link
Member Author

@JabRef/developers I need some help here. I want to execute the thing in a background thread and make it cancable. Looked already at some other, but it's not clear to me, why some of them are implemented in Frame and some in BasePanel. And I never worked with this AbstractWorker stuff. Any help welcome

@tobiasdiez
Copy link
Member

Like this

jabref/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java

Line 204 in 67e457c

BackgroundTask
?

@Siedlerchr
Merge remote-tracking branch 'upstream/master' into exportPDF
c89e6a0 
* upstream/master: (84 commits)
  Update README.md
  Update CHANGELOG.md
  Fixes #2789 Add Referer to API call (#2794)
  Change some FileDialogs to DialogService (#2767)
  Fix for issue 2762: Change CSV export to separate all names using semicolon (#2793)
  Set eclipse line wrapping to maximum
  Do not log an exception if side pane was not found (#2791)
  Added 'Ink' to the supported FileAnnotationType (required to close #2777)
  Renamed parseFileAnnotationType() to parse()
  Reimplement date editor in JavaFX (#2781)
  Update CONTRIBUTING.md
  Add new author
  Fixes handling of unknown PDAnnotation types.
  Update Checkstyle Version
  fix some more checkstyle warnings
  fix some more checkstyle warnings
  Fix Build failure, hopefully
  Spanish translation (#2773)
  Fixes #2766 If file is not found annotations might be null
  Fix language tests
  ...

# Conflicts:
#	src/main/java/org/jabref/logic/util/io/FileUtil.java
@Siedlerchr
try around with some background task stuf
42b0c62
@Siedlerchr
remove compile errro
353a1da
@LinusDietz
Copy link
Member

@Siedlerchr how would you describe the status of this PR? Is it feasible to include it into 4.0 or should we postpone it to 4.1?

@Siedlerchr
Copy link
Member Author

Siedlerchr commented Aug 8, 2017 via email

@LinusDietz LinusDietz added this to the v4.1 milestone Aug 8, 2017
@Siedlerchr Siedlerchr closed this Aug 22, 2017
@Siedlerchr Siedlerchr reopened this Aug 22, 2017
@Siedlerchr Siedlerchr closed this Aug 22, 2017
@Siedlerchr Siedlerchr mentioned this pull request Aug 22, 2017
Merged
6 tasks
@stefan-kolb stefan-kolb deleted the exportPDF branch August 25, 2017 11:27
@koppor
Copy link
Member

koppor commented Aug 31, 2017

This branch is followed up at #3147, because it was renamed to exportPdf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v4.1
Development

Successfully merging this pull request may close these issues.
5 participants
@Siedlerchr @koppor @simonharrer @tobiasdiez @LinusDietz