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

Broken view syntax for browsers on Windows #3

Closed
graphixillusion opened this issue Jul 23, 2024 · 7 comments
Closed

Broken view syntax for browsers on Windows #3

graphixillusion opened this issue Jul 23, 2024 · 7 comments
Assignees
Labels
bug Something isn't working windows Issues specific to the windows version

Comments

@graphixillusion
Copy link

graphixillusion commented Jul 23, 2024

The default config.toml has this config about the viewer:

viewer = [
    "firefox",
    "%p",
]

The problem with this config is that it doesn't do the job becouse firefox, like any other browser (atleast on Windows OS) wants this syntax:

file:///%p

So if the path is C:\Temp\test.md the right argument to pass to the browser is
file:///C:\Temp\test.md or file:///C:/Temp/test.md
(works both ways)

How to achieve this?

@Linus-Mussmaecher Linus-Mussmaecher added bug Something isn't working windows Issues specific to the windows version labels Jul 23, 2024
@Linus-Mussmaecher
Copy link
Owner

On Unix systems, the command line arguments on firefox work in such a way that calling

 firefox /home/linus/MyNotes/TestNote.md

will open firefox on the file URL file:///home/linus/MyNotes/TestNote.md. I'm very surprised that this does not work on Windows as well.
I will do some testing on a Windows machine and then report back.

My current suggested solution would be something like changing the creation of the command as follows:

  • Current behaviour: If any of the arguments in the config are exactly "%p", replace by the file path.
  • Planned behaviour: If any of the arguments in the config contain "%p", replace that appearance by the file path.

Then you could achieve your behaviour with the config viewer = ["firefox", "file:///%p"] as your suggested.

@Linus-Mussmaecher Linus-Mussmaecher changed the title View in browser is broken on Windows Broken view syntax for browsers on Windows Jul 23, 2024
@Linus-Mussmaecher Linus-Mussmaecher self-assigned this Jul 23, 2024
@graphixillusion
Copy link
Author

graphixillusion commented Jul 23, 2024

Well actually if i run this in a command prompt it works:

firefox c:\temp\test.md

but it doesn't work within rucola's view

@graphixillusion
Copy link
Author

graphixillusion commented Jul 23, 2024

Ok for this i think i have found the source of the problem: it doesn't work becouse the path passed is

c:\Temp\.html/test.html

Windows doesn't accept this kind of path: it must have all \ or all / not mixed. So these works

1. c:\Temp\.html\test.html
2. c:/Temp/.html/test.html

And this doesn't:

c:\Temp\.html/test.html

@Linus-Mussmaecher
Copy link
Owner

Linus-Mussmaecher commented Jul 23, 2024

In this case, what is your specified vault path in your config file?
If it's something like C:\Temp can you try to instead put vault_path = C:/Temp? Currently, the path to the HTMLs is found by prepending your vault path to /.html/<note-name>.html, so this might fix the issue.

If you are simply running rucola in your note folder without a configured vault_path, then this is probably an issue with std::env::current_dir() returning a path with \.
I will try to update the next version in a way that stadardizes use of \ and / across the program, but maybe the above helps for now?

@graphixillusion
Copy link
Author

I didn't specify any vault_path in the config. I've tried with the config you suggested but still the same result.

@Linus-Mussmaecher
Copy link
Owner

In this case, there must be some internal issue with the way rucola currently reads and build the paths.
I will have to do my own tests, and I'll inform you of any results once I have any.

@Linus-Mussmaecher
Copy link
Owner

This issue has been fixed by canonicalizing all paths before sending them to external applications (and thus converting to full \ on Windows and full / on Unix systems).

The next release, 0.3.4 coming probably tomorrow along with some other bug fixes, will contain this fix. No further action should be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows Issues specific to the windows version
Projects
None yet
Development

No branches or pull requests

2 participants