olivierkes / manuskript

A open-source tool for writers
http://www.theologeek.ch/manuskript
GNU General Public License v3.0
1.78k stars 237 forks source link

Importing images into Manuskript #593

Closed scribas closed 5 years ago

scribas commented 5 years ago

I tried to follow the instructions on the wiki page, for referencing the images inside the images folder: ![My Image One Title](images/myimage1.jpg). Firstly, it complained that it does not understand the protocol. I also tried various combinations in linux. I finally had to put the full path in, with a file:// upfront: ![My Image Title](file:///home/scribas/git_repos/manuskript/images/myimage1.jpg). I suspect that the reason for this is, that the present working directory is in the folder where the manuskript binary is kept?

Aside from this, if I put the correct reference into one of the pages within the Editor environment and hover over the link with my mouse, the image pops up. However, if I try and do this in the Characters environment, the image only flickers up for a second and then disappears again. As I move my mouse around over the link, the image will keep on flickering on and off very rapidly. It would be great if this could be improved?

gedakc commented 5 years ago

Following are some high level ideas for how to proceed:

I suspect that the reason for this is, that the present working directory is in the folder where the manuskript binary is kept?

You might try to confirm your suspicion by placing the image resources in the same folder. If this works then it might be a case of needing the code to prefix the currently open project folder with the relative image path.

You might try tracing the path through the source code to discover the section where images are displayed. As a hint for a starting place, the Manuskript main window is described in the manuskript/mainWindow.py python file.

For the second paragraph in the initial issue report, you might try to locate the code that displays an image while in the Editor pane (this pane used to be called Redaction), then find how this works in the Character pane, and then compare the two.

scribas commented 5 years ago

Feedback to date:

gedakc commented 5 years ago
  • I put a sample image into the same location as manuskript.exe on Windows and referenced it with: ![My Image One Title](file://example.jpg). That crashed Manuskript completely, without any error messages, as soon as I let my mouse hover over the image reference.

It appears that we were unlucky in trying this hypothesis and instead encountered a different problem. With that said there should still be an error message in the console log.

Where are you looking for the error message?

For example on Windows 10 when I run manuskript.exe (0.9.0) from the command prompt using the sample URL link you mentioned and hover over the URL link I see the following:

C:\manuskript-0.9.0-win32>manuskript.exe
Debug: Web rendering engine used: QWebView
Running manuskript version 0.9.0.
Found translation in settings:
Note: No translator found or loaded from system locale for locale en_US.
QWindowsWindow::setGeometryDp: Unable to set geometry 1112x808+8+31 on QWidgetWindow/'MainWindowWindow'. Resulting geometry:  1028x749+8+31 (frame: 8, 31, 8, 8, custom margin: 0, 0, 0, 0, minimum size: 498x441, maximum size: 16777215x16777215).
Loading: C:/Users/gedak/Documents/deleteme-1.msk
Detected file format version: 1. Zip: True.
Project C:/Users/gedak/Documents/deleteme-1.msk loaded.
Traceback (most recent call last):
  File "manuskript\ui\views\MDEditView.py", line 613, in finished
KeyError: PyQt5.QtCore.QUrl('file://image.jpg/')

C:\manuskript-0.9.0-win32>

A search through all the issue/PR reports shows that this problem was reported in issue #549 and fixed in the develop branch with PR #558.

  • I am still struggling to understand where the image gets called within the tool tip, when the mouse hovers over the image reference. Looks like I may have to do some reading up on how Qt works.

The error message in the console log above provides an excellent starting point. In addition, one can search the Issues and Pull Requests that might contain references to when a feature/bug was requested/reported, one can also search through the git log history to see if the programmers documented the feature/bug fix. There are also the other project resources, such as the web site and wiki, that one can search.

  • In general, it would be helpful if there was a way to map out the files and folders within a development project like Manuskript and explain a bit of the developer's thinking, to make it easier for newby developers to find their way. Is there such a mapping thing / documentation framework, aside from putting it in special wiki articles in Github?

If you discover "such a mapping thing / documentation framework" then please let me know. :-)

I think of each issue/bug/feature request as a puzzle to be solved. Sometimes there are many, easily discoverable clues to help solve the puzzle. Other times one must dig much deeper to find a solution.

One tip when developing Python code is to run the source code directly. On Windows 10 the command to invoke manuskript might look something like:

C:\Python37\python.exe C:\manuskript.git\bin\manuskript

scribas commented 5 years ago

Thank you for your patience. I am learning rapidly.

Okay, I am now running the newest development source files from git, instead of the Windows production binary. I can still open an image with a direct URL reference: ![My Image One Title](file:///Users/scriba_s/Documents/manuskript/example.jpg)

A relative URL still does not open, but at least does not crash the whole program: ![My Image One Title](file://example.jpg)

Finally, a relative URL, without the file:// portion, outright crashes the program: ![My Image One Title](example.jpg)

I can stop the error, by placing a try and except clause around lines 624 to 632 in manuskript/ui/views/MDEditView.py.

Error message:

Debug: Web rendering engine used: QWebEngineView
Running manuskript version 0.9.0.
Note: No translator found or loaded from system locale for locale en_US.
Loading: C:/Users/scriba_s/Documents/manuskript/test.msk
Detected file format version: 1. Zip: False.
Project C:/Users/scriba_s/Documents/manuskript/test.msk loaded.
Traceback (most recent call last):
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\ui\views\MDEditView.py", line 624, in finished
    pos, url = ImageTooltip.processing[reply.request().url()]
KeyError: PyQt5.QtCore.QUrl('')
Fatal Python error: Aborted
Current thread 0x00008ba4 (most recent call first):
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\main.py", line 145 in launch
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\main.py", line 158 in run
  File ".\manuskript", line 13 in <module>
gedakc commented 5 years ago

'Sounds like you are making headway now. :-)

gedakc commented 5 years ago

How are you making out @scribas?

scribas commented 5 years ago

I did not make any further progress and got swamped with work. Here is what I tried:

Firstly, I tried to figure out unsuccessfully where the current pwd is pointing, with a number of print statements to print to console. I also tried to understand, whether there is possibly an existing variable somewhere in the other files, which contains the path to the working project folder. However, it is pretty difficult finding my way among the thousands of files and how I would even bring such a variable into MEditView.py. The fore-mentioned illusive tool that maps out functions within the directory maze would really come in quite handy...

I also do not know PyQt and am not sure how to add the project path to the correct position, where the image gets displayed. I have some vague ideas, but not 100% sure how the associated code all works (seem to be quite a number of things happening, setting up caches, etc). I suspect that I would do better, if I had some experience with PyQt. I did considered going down that road, but am already busy with a number of Udemy courses for other subjects.

Finally, I went into the git history and found that worstje was the last person to work on manuskript/ui/views/MDEditView.py. I thought about this and came to the conclusion that, surely, s/he must have tested their work, before logging their pull request and declaring that the bug has been fixed? So, I am a bit worried that there is some really simple way of referencing the images?

gedakc commented 5 years ago

There is a lot to learn. :-)

I am far from an expert in Manuskript and the languages used. With that said I can try to provide some thoughts on how I might approach this challenge.

Hopefully these suggestions help.

worstje commented 5 years ago

I only just now came across this PR while looking up something else... and I happened to notice someone brought up my name by coincidence.

@scribas I am sorry you've been having problems. I assure you: if I had spotted any problems with my PR, I would not have sent in the pull request. On my end (using Windows 10), the reported bug was fixed and that which worked previously appeared to still work. But it was not a complete rewrite by any means; it merely adjusted some logic oversights.

From what I recall, PR #558 was about a mismatch between the URL requested and the URL retrieved, which caused the code to crash. At its very core, this is a different bug involving remote resources and does not involve any sort relative path specification.

To be even more blunt: the image tooltip code does not concern itself with the nature of the link in the slightest. Everything is wrapped into a QNetworkRequest and passed off to QNetworkManager to handle. From my perspective, it makes sense for it to choke on a relative link because it might not be considered a 'network resource'.

Which brings me to my pull request from a few months ago: while I do not believe it broke any functionality that already existed, I also did not test whether local / relative paths were broken. Whether or not it is supported to begin with, and whether we want it to be supported from here on forth, is something that needs to be considered. (Edit: Since it was listed on the wiki, I assume local relative access is something we want... but I missed that when I skimmed this topic before. And I definitely didn't know about the scope of the links myself; I thought 'online' paths was the full reach based on what I saw in the tooltip code.)

I'm willing to look into the matter since I still kind of remember this nook of the code, but if I do, I'd love to have a clear understanding of the sort of links that should work. Keep in mind that this program has to work in Windows, Linux and Mac, and as such whatever 'fault' conditions may exist might be specific to only some of those, especially given the fact that Windows file:// links also include a drive letter which makes it difficult for me to use your example links to trivially reproduce the crash. (Honestly, given this fact, I would not be surprised if there is some sort of Qt bug we are running into here; it wouldn't be the first time...)

But at the very, very least we can add some more try-except/finally statements around the code in the manner you have already done to track down the problem yourself; that is a defensive bit of coding that I should have done last time I touched the code. So I'll go and see what I can do; maybe I can find a moment to squeeze in a patch for this before 0.10.0 comes out.

In the future, please drop a mention using the at character (@) so that the person you refer to gets a headsup. You would probably have heard from me a bit sooner then. 😄

worstje commented 5 years ago

Please test PR #629 to see if it addresses the issues discussed in this topic.

As it is right now, the PR in question is not 100% ready for inclusion yet (there's some changes I want to try and test when I am properly awake and motivated), but knowing whether or not the document with testcases covers every scenario that needs covering, as well as whether it works as desired on whatever operating system(s) you are using is key in making sure I don't have to come back for a third round on fixing this code. 😜

scribas commented 5 years ago

Thanks @worstje, I have been totally snowed under over the last couple of weeks. Will definitely try and pull the newest version and see whether it works. That would be amazing. Will have a look what changes you made and see whether I can learn from it.

scribas commented 5 years ago

I pulled the latest commit and tested again on Windows 10 (I can test on Linux Mint at home, once Windows 10 passes). I am not 100% sure where in the project folder tree the image should now be placed, but went ahead so long and used the following statement: ![My Image One Title](file://example.jpg). This still brings up the following tooltip message: Error opening //example.jpg/: The specified path is invalid.

If I remove the file:// portion and rather use the statement: ![My Image One Title](example.jpg), manuskript crashes totally with the following error message:

Debug: Web rendering engine used: QWebEngineView
Running manuskript version 0.9.0.
Note: No translator found or loaded from system locale for locale en_US.
Loading: C:/Users/scriba_s/Documents/manuskript/test.msk
Detected file format version: 1. Zip: False.
Project C:/Users/scriba_s/Documents/manuskript/test.msk loaded.
Traceback (most recent call last):
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\ui\views\MDEditView.py", line 624, in finished
    pos, url = ImageTooltip.processing[reply.request().url()]
KeyError: PyQt5.QtCore.QUrl('')
Fatal Python error: Aborted

Current thread 0x0000a6d4 (most recent call first):
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\main.py", line 179 in launch
  File "C:\Users\scriba_s\Documents\git_repos\manuskript\bin\..\manuskript\main.py", line 192 in run
  File ".\manuskript", line 13 in <module>
worstje commented 5 years ago

@scribas You are not using the intended fix for the pull request; the line numbers do not match up nor can the error you list happen any longer in my suggested change.

To get the PR in question, execute:

git fetch origin pull/629/head:pr-629

Next, to switch to the changes of the PR, execute

git checkout pr-629

(Once done, you can use git checkout develop to go back to the main develop branch.)

See this link for more information on what those commands do.

Please post any further discussion regarding the PR code in the Pull Request so that the discussion of the proposed code changes is all in one place.

Regarding relative paths functionality

What the code does is ask Qt to make sense of the entered text. It does this based on the current directory (pwd) of the process in question. Because Manuskript does not adjust this anywhere, this means it always matches the directory you start out with. Given the most commonly used starting commands

py bin/manuskript.py (Windows) bin/manuskript (Linux)

that means the current directory is relative to this location. If you'll look at the test sheet included as an attachment with the PR, you'll see a number of tests that try to access one of the icons that are a part of Manuskript, which are somewhere in the icons/Manuskript directory IIRC. That file also contains my results of testing different syntaxes, so you might want to refer to it to get an idea of what code is supposed to work.

To be quite honest: I am not quite sure why we want to support a relative syntax in links. Keep in mind that whatever code Manuskript produces is going to be exported to a different program, and that program is unlikely to do more than copy the link.. which means that for any other person, those links are highly likely to be broken if we are indeed referring to a local file. The only situation in which I could see it be remotely useful is when the final format directly includes the image into its own file so that it becomes a resource as a part of the export itself.

Also note that it is not trivial to have intuitive support for images inside Manuskript at this time. We could likely change the current directory to be where the .msk resides, but that is a functionality change that is out of scope for a bugfix for the upcoming 0.10.0 version. There is no way to truly integrate images into the Manuskript project itself and also have QUrl load them nicely because we might be using the single-file format (a renamed .zip) that is inaccessible to it.

So at present my recommendation is to stick with absolute paths to avoid unexpected behaviour, or to (for now) be aware that any relative paths are likely (but not guaranteed) to be relative to the directory manuskript is installed in.

obw commented 5 years ago

Well actually I don't use Images in my Manusscript Projects (They are two big one (more then 500 Pages) and one smaller one (done with around 300 Pages))

But it's looks like in half a year from now, I have a collaboration with an Illustrator, he works under MAC I'm under Linux (Kubuntu 28.04 LTS), exchange over SVN, we both will also write in Manuskript. The Path under MAC will be totally different then under Linux!

So e relative Path handling will be needed.

I see two simple ways too do it:

  1. add a 'Varable' for the Projectpath like '$project'
  2. add automatic the Projectpath to all relativ paths

I prefer the 'Varable' way, because we can add something like a Var replacement System later:

worstje commented 5 years ago

There is a lot to take in and respond to here, and I don't even know where to begin... 😯

Why some changes are not trivial

Manuskript fulfills the role of a writing environment. However, the syntax of what is written is up to you, and eventually interpreted by whatever exporter you use. Maybe you use a Manuskript builtin exporter, maybe you use pandoc, or maybe you are like me and have an external script to fulfill that job because Manuskript just isn't good enough for your exporting needs yet.

The sort of variable you mention could be implemented in many ways. Ideally (from a writing-in-Manuskript perspective) you would have a window as you say, and Manuskript would substitute the correct string before handing it off to the target. But life is not that simple: depending on the format you yourself author in, such variables might clash with language features itself. Hell, what if someone is writing a technical document regarding programming and version control, and those variable names conflict? That would be really troublesome to handle for them.

More sensible from a Manuskript perspective is to stay out of the entire 'variable' business, and to leave any 'logic' to whatever exporters are activated. After all: they know the traits of the authoring language, as well as the way the content will come out. And especially for our current topic, the latter bit is super-important.

Suppose you are authoring everything down to a single pdf. At that point, all the relative links would be grounded on the same path. As for any images, they are imported into the pdf file itself, so no more paths exist.

But if you are authoring a website manual, then you might have several layers of nesting depending on the section you are working on. In this case, the relative path of any links or images is based on the location of the produced file, and never a part of the file we export.

These are two very opposite ways to use Manuskript, but we are not specific to one or the other.

Finally, there is the 'problem' of whether or not image files are actually turned into a part of the project or not. Right now, this is not really the case most of the time: people cannot edit the files in Manuskript so it serves no purpose. But even if they are, there is the complications it brings along on a lot of different levels:

Suffice to say: this sounds like a simple wish, but it has tons and tons of complications that will impact Manuskript as a whole... and unless someone inventories really carefully what needs Manuskript is meant to do and how to minimize the impact of such changes, it will not be a good change for 99% of the users at all. (Keep in mind we also try to run on 'light' devices like netbooks which have very strict limitations compared to a desktop.)

What you should and/or can do

I had my own challengess when writing my project in Manuskript, and the answer of one of them was to realize Manuskript does not yet meet all my needs. This on its own was not a big problem (I coded an external script to handle my exporting which met my exact needs eventually) but this would have been a lot easier if I had known the limitations I was dealing with before I started writing.

My suggestions is to create an example project that contains the basic format you need, and start fussing to see if you can come to produce the desired output format long before you have tons of writing in there that requires detailed fiddling to get right in different spots. It is better to have one good sandbox to test with than your important work-in-progress.

While doing this, start studying the tools available to you to a crazy degree. Your limitation while writing might in some ways be Manuskript because it is what you write in, but when it comes to the actual export, Manuskript is not very mature. If you look at a document converter like pandoc, you'll come to realize it has far more knobs with which you can control what is going on.

During this phase, it is key that you understand your tools, and in what ways they are limited.

Oh right, variables and paths.

But for now, I recommend just using the capabilities you have available already, which is the directory you start Manuskript in:

cd /home/me/project_images manuskript

That should cover most of your practical problems:

  1. the images directory can change and be anywhere
  2. as long as whatever exporting method respects the current directory, it will be able to find your images
  3. it already exists and is thus free of newly-invented technical complications.

Alternatively, if you insist on really having some sort of variable that is interpreted by your exporting process, look at the string replacing functionality inside of the exporter. (I personally don't like the exporter because of a number of practical limitations I ran into on my project, so for me this was not really an option.)

If you end up going down the deep end with pandoc, explore how YAML metadata blocks can help you. I use this format at the top of my chapters to specify chapter metadata that tells me 'perspective is from this character, this is the location it is happening at, etc'. Since I rolled my own exporting script, I reinvent the wheel a little bit, but it was the flexibility I needed. (If in the future Manuskript comes to support it, it would also be possible to pass custom metadata into pandoc without writing such YAML blocks into your own document.)

But know that the more complicated solutions are definitely not going to play nice with the convenience of the Manuskript tooltip; the only way to really know if something works is to just export it and find out for yourself. 😄

gedakc commented 5 years ago

Closing this issue because PR #629 has been merged with the develop branch which prevents crashes when hovering the mouse over image URLs.