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 option to set the temporary directory path on Windows. Also a few smaller changes. #86

Closed
wants to merge 5 commits into from

Conversation

MaxKoll
Copy link

@MaxKoll MaxKoll commented May 12, 2021

Hey,

I ran into the same issue as described in #15, #32 and #56 (even after #81 & #82). So I started digging and came up with a solution/workaround.

What's the problem?

As of May 2020, MiKTeX does no longer support short filenames (aka 8.3 filenames). Unfortunately in my case the path to the temporary directory as given by Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties).get("TmpD", Ci.nsIFile).path includes short filenames.
If I run the command that LaTeX It! shows in the debug box manually (with the paths adjusted), everything works fine.

What I considered:

I tried finding a reasonable way to extract the full path in the given framework, but had no success:

  • Use Cc["@mozilla.org/process/environment;1"].createInstance(Ci.nsIEnvironment).get("TEMP"/"TMP") instead, but it also contains short filenames.
  • Use a different file or output stream interface that can return a reconstruction of the given path. I have found no such thing. I learned that nsIFile does not exactly represent a file (handle), but a string (the path). As such, many of its properties and methods are just string manipulations. A notable exception is exists() which is indeed a true file operation.
  • Use a shell script to construct and push the full path into a file or to the registry and get it from there. The shell script itself would be rather sophisticated (examples on StackOverflow) and the "solution" is so hacky that I didn't even try it.

Also, I haven't found a way to pass a working/execution directory to nsIProcess. Otherwise you could just launch the executables from the temporary directory.

What I came up with:

I added the option to change the path of the temporary directory. This setting is only used and shown on Windows.

During this I discovered a few other issues that I also tackled:

  • Options: The contents of the fontpx_textbox are not displayed.
  • Options: The file pickers are associated with the input boxes by order of appereance in the code, which might break if the code is restructured.
  • Firstrun: The result of async function file_exists() is used in expressions without await. As the result is a promise, it is always true.
  • Main: When compiling LaTeX on windows, multiple command windows pop open, but it should be a silent background process.

I tested my changes on:

  • Windows 10 Pro (20H2), Thunderbird 78.10.1, MiKTeX 21.2
  • Xubuntu 21.04, Thunderbird 78.8.1, TeX Live 2020

On these systems, everything seems to work fine. I don't have the possibility to test on OS X.

If some or all of these changes are desirable, please feel free to use them any way you like. Please tell me if you want this PR split up.

Max

@sphh sphh requested a review from protz May 12, 2021 21:01
MaxKoll added 5 commits May 12, 2021 23:37
The size property is not meant for inputs of type number. Inside the box arrows are rendered that interfere with the text content.

Fixed width of 6em looks fine for 2-digit numbers on Windows and Linux.
file_exists() is defined as async function, i.e. returns a promise (which is always truthy).
This is probably only necessary for Windows, not Linux or OS X.
As of May 2020, MiKTeX does no longer support short filenames, so if the path to the temporary directory contains short filenames, LaTeX compilation will fail.

In the current framework it seems to be impossible to obtain the full path with only long filenames easily, so present the user with an easy way to change it if necessary.

Also display the "auto-detected" path on the firstrun page.

On OSs other than Windows this option is unset, unused, and hidden in the options and firstrun pages.
@MaxKoll
Copy link
Author

MaxKoll commented May 17, 2021

I think I found a better way to do this:

Don't call latex.exe directly but via shell (same as with dvipng.exe), and change into the temporary directory first by prefixing the command with cd /d [temp_path] &&. I already tried it and it seems to work as expected. No need to set the temporary directory manually.

I will soon open another PR with the proposed changes.

Also, I realize, I should have pushed these changes to a separate branch associated with this PR. If I close this PR, is it still tracking changes to my forked mailextension branch? (I hope not.)

@sphh
Copy link
Collaborator

sphh commented May 17, 2021

Do those two help pages help?

  1. https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/closing-a-pull-request
  2. https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request

I believe the brûte force way is to close the pull request, delete your fork (that's somewhere in the settings of your fork) and start again from square one ;-)

@MaxKoll
Copy link
Author

MaxKoll commented May 19, 2021

Thanks! I will close this now and then reset my mailextension branch, which will hopefully lock this PR's state. Fingers crossed. :-)

@MaxKoll MaxKoll closed this May 19, 2021
@MaxKoll MaxKoll deleted the mailextension branch May 19, 2021 16:30
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.

2 participants