| | <<<Back 1 day (to 2018/02/11) | 20180212 |
| paulgardiner | Please can someone review a tiny change on paulg/master. It fixes annotations failing to be marked as dirty when altered by javascript and UI events | 10:57.47 |
| tor8 | paulgardiner: LGTM. | 11:05.00 |
| paulgardiner | Thanks tor8 | 11:05.14 |
| sebras | tor8: is there a reason we use the name __printflike in include/mupdf/fitz/system.h on line 185 to get printf complaints from gcc using __attribute((__format__))? | 13:12.45 |
| | tor8: malc_ mentioned that we are using a reserved namespace starting with __, and we are. but I'm suspecting we are implementing an attribute that might be defined in some headers somewhere. I haven't found where though. | 13:13.34 |
| | it was introduced back in 2008 in commit 4b9181cdb5 | 13:13.45 |
| | would it e.g. be possible to name it..PRINTFLIKE() and avoid using a reserved namespace? | 13:14.22 |
| malc_ | sebras: tor already did it | 13:14.31 |
| | FZ_PRINTFLIKE | 13:14.34 |
| sebras | oh, there you go. | 13:15.14 |
| | I guess I should just shut up and fetch. :) | 13:15.24 |
| malc_ | ;) | 13:15.43 |
| | sebras: regardless, tack for bringing it up | 13:29.03 |
| tor8 | sebras: now that you're here and have looked. are the commits on tor/master good to go? | 13:54.13 |
| sebras | tor8: give me a second. | 14:06.20 |
| | tor8: I'm thinking perhaps you want to check if showsearch is != 0 for KEY_ESCAPE? | 14:10.18 |
| | tor8: not a big deal but I anticipate escape might be used for annotations later. | 14:10.31 |
| tor8 | if (showsearch) already steals the keyboard input by taking the ui.focus | 14:12.07 |
| sebras | ah, I didn't notice that. | 14:12.25 |
| tor8 | I expect ESC to take you back to normal mode | 14:12.27 |
| sebras | yeah, most likely in all cases (search, annotations, whatever) | 14:12.48 |
| | tor8: PDF_PERM_NOTES -> PDF_PERM_ANNOTATIONS? | 14:13.14 |
| tor8 | sebras: I just copied and pasted, but you're right we could probably improve them while making them public | 14:13.54 |
| sebras | tor8: my thinking exactly. | 14:14.12 |
| | tor8: I think the PDF permission bit thing is ok to, and I'd leave the permission dictionary thing for another commit. | 14:14.56 |
| | have we actually ben asked for the permission dictionary thing yet? | 14:15.12 |
| | as I understood you it is a bit tricky so perhaps we could wait with that until it is actually requested/needed. | 14:15.43 |
| tor8 | sebras: yes, see the "Retrieving PDF specific permissions from MuPDF" email to support | 14:15.55 |
| | sebras: ugh. bit 6 is more than just annotations. these permissions are icky. | 14:17.50 |
| sebras | tor8: true, I only read the first part. :-/ | 14:19.34 |
| | tor8: perhaps ANNOTS_AND_FORMS then. :) | 14:19.48 |
| tor8 | PDF_PERM_ADD_OR_MODIFY_TEXT_ANNOTATIONS_AND_FIELDS is a bit long... | 14:20.10 |
| sebras | tor8: I read the email but it was you who brought up the PDF 1.5 dictionary, and I don't see a request for support for it yet. | 14:20.25 |
| tor8 | that one could be handled behind the scenes by the new function, should the need arise | 14:20.40 |
| sebras | tor8: though designing the API for it is probably wise (probably just a matter of time before we get a request) | 14:20.47 |
| | yeah, so I'd simply remove the TODO for now. | 14:21.21 |
| | tor8: as I read it the question is only about retrieving the current set of permissions. not setting them or having them checked. | 14:22.35 |
| | tor8: so I would say LGTM in addition to what I wrote above. :) | 14:23.30 |
| tor8 | sebras: okay, revised tor/master | 14:26.49 |
| sebras | tor8: hm. pdf_document_permissions() exists, but we also have a fz_has_permission(). which interface are we advocating for? | 14:43.55 |
| | FZ_PERMISSION_* doesn't have all of the insane PDF permissions though. | 14:44.13 |
| tor8 | fz_has_permission is the generic one, that we've always had | 14:58.15 |
| sebras | yeah, does it match EPUB? | 14:58.24 |
| tor8 | but there are funny weird pdf specific permissions that this customer wants to look at | 14:58.33 |
| sebras | I'm having a hard time finding permissions in the EPUB spec. | 14:58.34 |
| tor8 | sebras: none of the other document types actually have permission flags that we look at :) | 14:58.52 |
| sebras | tor8: oh. :) | 14:58.59 |
| | tor8: that explains it. :) | 14:59.04 |
| tor8 | but if they do, I would anticipate the fz_has_permissions api to be sufficient | 14:59.17 |
| sebras | tor8: ok. then LGTM on tor/master. | 14:59.32 |
| tor8 | thanks. | 14:59.38 |
| Robin_Watts | tor8: If memory serves, you'd LGTM'd the begin_layer/end_layer, hadn't you? | 15:45.13 |
| | oh, no, we need java and mutool run bindings for it. | 15:46.51 |
| sebras | tor8: is tor/ui a preparation for annots? | 16:15.49 |
| | tor8: Robin_Watts: do either of you mind reviewing sebras/master? | 16:24.58 |
| Robin_Watts | sebras: Let me look. | 16:25.09 |
| | We used to only load the globals if they were indirect. | 16:26.30 |
| | We now will load them whether they are direct or indirect. | 16:26.42 |
| sebras | Robin_Watts: wait... I suddenly remembered that tor8 mentioned that I ought to have marked the global stream instead. d'oh. | 16:26.49 |
| Robin_Watts | sebras: I was going to say that our standard mechanism for this is pdf_obj_mark (or whatever it is). | 16:27.17 |
| sebras | Robin_Watts: yeah, but whether they are indirect or not is not important, the question is whether it is a stream. | 16:27.19 |
| Robin_Watts | ok., | 16:27.28 |
| | Second commit lgtm. | 16:27.43 |
| sebras | Robin_Watts: and with tor8's recent changes to pdf_is_stream() is asks for pdf_get_indirect_document() which ought to return NULL for objects that are not indirect. | 16:28.04 |
| Robin_Watts | I wonder if we should be internationalising our errors... | 16:28.18 |
| sebras | (additionally it also checks the stm_ofs in the xref) | 16:28.20 |
| | Robin_Watts: perhaps when someone asks for it..? | 16:28.34 |
| | Robin_Watts: I'm not sure if anyone is actually using the option to receive mupdf's error messages and show them in a dialogbox (except for our own viewers) | 16:29.02 |
| | Robin_Watts: gettext is ugh, btw. | 16:29.18 |
| Robin_Watts | gettext being a lib for internationalisation? | 16:35.19 |
| | I was imagining that we'd roll our own. | 16:35.43 |
| sebras | Robin_Watts: it is. | 16:35.55 |
| Robin_Watts | but very low priority. | 16:36.08 |
| sebras | Robin_Watts: basically you surround every textstring with _("hello") and then a tool extracts these from the source code. | 16:36.21 |
| Robin_Watts | sebras: I was thinking we'd do it epage style. | 16:36.42 |
| sebras | Robin_Watts: the problem is that we do propagate the error strings from the libraries we use too. how would we handle those..? | 16:36.42 |
| | Robin_Watts: enlighten me. :) | 16:36.49 |
| | Robin_Watts: error codes? | 16:36.55 |
| chrisl | Isn't gettext GPL? | 16:37.42 |
| Robin_Watts | Epage does stuff like Error_create(context, Module_Error, bar, baz); | 16:37.50 |
| sebras | chrisl: quite possibly, so there might be licensing issues too. | 16:38.16 |
| | Robin_Watts: so bar and baz are strings? | 16:38.33 |
| Robin_Watts | and each module has a list of strings: LoadFailure: Failed to load file %s version %d | 16:38.43 |
| | so bar/baz can be any type. | 16:38.56 |
| sebras | ah, LoadFailure is the Module_Error thing you mentioned above? | 16:39.13 |
| Robin_Watts | LoadFailure would be 'Error'. | 16:39.25 |
| | Error_create(context, DocumentLoader_LoadFailure, bar, bar) | 16:39.46 |
| sebras | Robin_Watts: that also means we would need to translate all error messages/warnings before every release. | 16:40.52 |
| | Robin_Watts: to make it useful. | 16:40.59 |
| Robin_Watts | If we're smart we can avoid doing the string formatting unless required. | 16:41.00 |
| | sebras: We can invite people to submit translations :) | 16:41.15 |
| sebras | Robin_Watts: yeah, but these are warnings/error cases so they ought to be rare. | 16:41.25 |
| Robin_Watts | (If you don't have a translation, use the fallback/english one) | 16:41.40 |
| sebras | Robin_Watts: of course. | 16:41.48 |
| Robin_Watts | Anyway, like I say, a low priority. | 16:42.03 |
| sebras | Robin_Watts: mmm, you'd need another argument for ERROR_GENERIC, ERROR_SYNTAX, ERROR_ABORT, ERROR_TRYLATER though. | 16:42.37 |
| | or perhaps that would be implicit in DocumentLoader_LoadFailure some how. | 16:42.53 |
| Robin_Watts | sebras: Yeah, I was just saying what Epage did. | 16:42.58 |
| chrisl | Or accept that no one reads errors/warnings anyway! | 16:43.47 |
| malc_ | sebras: i'd argue that fallback should be german (after all it's never your fault given verbs like http://dictionary.reverso.net/german-english/umfahren) | 16:43.47 |
| sebras | Robin_Watts: new version up on sebras/master. | 17:40.33 |
| | Robin_Watts: this one clusters well. | 17:43.10 |
| Robin_Watts | looks simpler. | 17:43.54 |
| | Didn't the previous one fiddle with some error handling too? | 17:44.25 |
| | (pulled some stuff into the fz_try) | 17:44.33 |
| | but all 3 lgtm. | 17:45.13 |
| sebras | Robin_Watts: it did, but I concluded it was not necessary. | 17:59.13 |
| tor8 | I'm with chrisl -- our error messages are technical details, that nobody would read anyway. | 18:42.06 |
| malc_ | tor8: for the logs: mupdf/include/mupdf/pdf/crypt.h:39:2: error: ISO C restricts enumerator values to range of 'int' (4294967292 is too large) [-Werror,-Wpedantic] | 23:51.17 |
| | PDF_DEFAULT_PERM_FLAGS = 0xFFFFFFFC /* all permissions granted, reserved bits set appropriately */ | 23:51.17 |
| | | 23:51.17 |
| | Forward 1 day (to 2018/02/13)>>> | |
Log of #mupdf at irc.freenode.net.