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.

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

Add publish action for PDF export #2618

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

dhmint
Copy link

@dhmint dhmint commented Jan 3, 2021

Adds a publish action along with menu item and timer callback which can be configured in settings. Publish is essentially ExportAsPdf except that it is a silent operation (no user interaction, file choosing is done automatically like autosave) and that it optionally executes a program/script on the exported file. We have written this extension for online courses in math and computer science and think it would be of interest to other users as well.

Copy link
Member

@Technius Technius left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Related: #2363

Something like this is better implemented as a plugin (input wanted from @rolandlo), although we would need a minor addition to the plugin API. This would avoid having to add a separate publish action, which will just cause confusion in the future.

It doesn't make sense to add a silent flag to showFileChooser when the whole point of showFileChooser is to, well, present a dialog for the user to select a file.

Thoughts?

/**
* The publish handler ID
*/
int publishTimeout = 0;
Copy link
Member

Choose a reason for hiding this comment

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

This name is confusing; I initially thought this was the publication interval. Can you rename this to something like autoPublishHandler?

@rolandlo
Copy link
Member

rolandlo commented Jan 7, 2021

Something like this is better implemented as a plugin (input wanted from @rolandlo), although we would need a minor addition to the plugin API. This would avoid having to add a separate publish action, which will just cause confusion in the future.

In principle I agree it makes sense to implement this as a plugin. It will need a different UI though and there is one issue, which may be quite non-trivial. Suggestion/questions:

  • I would add an app.ExportAsPdf(filename) command to the plugin API, where all you need to specify is an absolute filename.
  • The filename can be generated flexibly by the plugin. The plugin should probably have access to the filename of the xopp-file. So should we add another API command app.getFilepath() or should this be part of app.getDocumentStructure()?
  • Code to execute after the export could be written in Lua as well (using the os.execute or io.popen for system calls, but also capable to interact more closely with Xournal++ than pure system calls). However how should we make the code only run after the export job has been finished? Currently this happens in PdfPublishJob::afterExport(). That looks non-trivial.
  • Via the plugin we would need to create an independent little GUI, which we can do using the Lua lgi-module. If you have Xournal++ cover the full screen, this plugin GUI would have to be focused again if you want to change the settings or trigger a "publish" action.
  • Should the settings (for the time interval), be saved in an external file (or is it fine if the time interval is forgotten when restarting Xournal++)?

@LittleHuba
Copy link
Member

However how should we make the code only run after the export job has been finished?

Is it possible to provide a callback in the plugin that can be called from our side once the export is finished?

@rolandlo
Copy link
Member

However how should we make the code only run after the export job has been finished?

Is it possible to provide a callback in the plugin that can be called from our side once the export is finished?

In principle, this should be possible, sure. The plugin would have to register a listener for this signal and then we could call the function. It means the beginning of the introduction of a signaling system, extending our home grown plugin engine. This means some work, but also benefits with regards to other use cases of plugins.

@bhennion
Copy link
Contributor

I have given some thoughts to this. I think an ideal Publish plugin/job should

  1. Allow for exports in every supported format.
  2. For formats allowing it, only export the pages that have changed since the last publication (to avoid further lags like in Temporary input lag when autosaving a big file #2675)

A use case of mine: publish my notes as a webpage. Each page is a small png file updated only when needed.

Should the plugin callback function be implemented (as suggested by @LittleHuba), then the first item will not be an issue. The second point however, requires a up-to-date hasPageBeenModified flag exposed to the plugins.

Would that be doable?

@rolandlo
Copy link
Member

I have given some thoughts to this. I think an ideal Publish plugin/job should

1. Allow for exports in every supported format.

2. For formats allowing it, only export the pages that have changed since the last publication (to avoid further lags like in [Temporary input lag when autosaving a big file #2675](https://github.com/xournalpp/xournalpp/issues/2675))

A use case of mine: publish my notes as a webpage. Each page is a small png file updated only when needed.

Should the plugin callback function be implemented (as suggested by @LittleHuba), then the first item will not be an issue. The second point however, requires a up-to-date hasPageBeenModified flag exposed to the plugins.

Would that be doable?

@bhennion It would be easy to add a new document listener, that stores (for each page) the time, when it has last been updated. The plugin API can provide a function exposing these update times and the plugin can check for pages that need update in regular time intervals (for example once each second). Does that sound like a good solution to you? It would be easier than to notify the plugin directly about page updates.

@bhennion
Copy link
Contributor

@rolandlo yes, that sounds like a better way to so that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants