Search Local Documentation

[AvinZarlez]

As requested in #7

I'd like somebody to test this on a Mac and Linux, if possible.

Specifically, can you try setting unity-tools.localDocumentationViewer to "" and see if it works. On windows, it does not, so you have to specifically input your browser.

Normally, opening a url from command line opens the default browser. But if opening a local html file, it will automatically parse the query from the URL (thus opening the search page without the desired search). Opening the specific browser will retain the URL query.

Any thoughts on this as well? Bit of a niche feature and not exactly easy to understand. Don't know if I should add more documentation right up or not worry about it, the few people who care about local docs can figure it out?

[AvinZarlez]

Also I am aware this breaks unit tests, but it does so for a dumb reason and it's not this fixes fault.

I will fix the issue in master and rebase this later before merging in. When testing locally, just ignore the unit test failure and debug anyway

[AvinZarlez]

Fixed testing issue, branch now passes checks

[rostok]

I'd love to see this feature added. In my fork I just changed the unity_search variable in search.ts and added appropriate config setting. I work on windows and this does job. Isn't this sufficient?

[AvinZarlez]

That's the gist, I just didn't didn't get around to testing this to make sure my implementation was good. You can see the changes in this PR are similar to what you have. But I really ought to settle this and merge into the master branch.

[AvinZarlez]

@rostok I could not get to work on my machine using the way I see from your fork. It will load the local web page, but the search query is missing. Is yours working?

See https://github.com/microsoft/vscode/issues/75356

[AvinZarlez]

What's the exact value you load into your altSearchDocsUrl setting?

[rostok]

I feel awkward because I cannot confirm it working. I would have sworn it was. Anyway the result is exactly the same as you describe - local file is opened without question mark and GET parameters. Will look into it.

[AvinZarlez]

VSCode closed my issue as it's apparently an electron issue...

Which means we have a few options:

1. Wait for electron to fix this issue and the upstream fix to be added to VSCode [ Also known as "do nothing" ]
2. Change the scope of the feature to just open local documentation, but without selection. Much less useful of feature.
3. Add a package that supports opening a local webpage with query. Removed opn not too long ago, not sure if that has similar issue or not. Given all of npm I'd imagine there is something that works out there.

I'm not sure which of the three I like the most.

One is the cleanest solution, and honestly not sure what the demand for this feature is. But it also might not happen or not happen for a long time.

Two can be done super quick, but not a really compelling feature to just open the documentation for you without being able to search selected text.

Three adds the feature, but also a new package to the project. Probably best once we find a good package? I don't think people would be too upset by bigger extension size. Besides, can always go to option one if this bug ever gets fixed.

[rostok]

All right. I got this working with child_process exec. Frankly don't know if that's acceptable, but well, it works. Won't make pull request yet but please take a look into this https://github.com/rostok/unity-tools/commit/c002dfc0e31bbbda2ced134d7aada7590f2f2ba5

[AvinZarlez]

Having to put the full file path to the browser is not a very elegant ask. I'm hoping we can do it a better way than that. Not to mention to test to make sure this works on Mac/Linux too...

Also, FYI, check out what I have on this branch now. Right now your implementation would have issues if you changed the altSearchDocsUrl settings between uses because of where you put the git. Not to mention introduce more chance of errors by replacing unity_search as opposed to using a new variable as I have in this branch.

I'll have to look around and see if child_process is the way to go, but I feel like I looked at that before and it's not cross platform compatible in the same way as opn npm package

[rostok]

I agree that's not the perfect solution. However full path is only needed if browser executable path is not in a PATH var. I am not sure how Windows opens default application for file:// protocol or any other non exec file but opening file url will always truncate GET parameters along with question mark. Results for the opn (now open) package are exactly the same, probably by falling back to OS handling of such case. Guess this is not a great insight but I would advocate for child_process anyway.

A checked the current searchLocal branch (5723eca) but couldn't get it working. Nothing happens on Ctrl+' or via command palette (Win10).

I will also try to look for substitute with vs code tasks.

[AvinZarlez]

My branch does what I described in https://github.com/microsoft/vscode/issues/75356 , meaning that nothing will load. If the code is modified to not include the query, it will load the webpage without it.

My initial experiments with this branch specifically used opn but had to set a explicit choice of browser to open it in. Didn't need the file path to that browser, just like "firefox". I made a setting for that value, but seemed like it wasn't an elegant solution. I also never got around to testing on Linux/Mac.

I know a lot of users of this extension are cross platform. Specifically with Mac, don't know how many Linux there are out there but I'd assume much less.

I'm not saying can't use child_process, but might be a better way out there.

This week I'm busy with another work project so don't have time to read up and find the ideal solution

[AvinZarlez]

From https://github.com/electron/electron/issues/18851

"This is 100% expected behavior, file paths are not URLs. Just because some browsers allow you to load files does not mean they are URLs. For instance test.html? is a perfectly valid filename on windows, launching a local file with query parameters does not make sense from a filesystem perspective"

So I am thinking the only way to accomplish this feature is to have the user explicitly set the browser, so can pass a file:/// URL directly to that. As would be done in a child_process.

To do:

• [x] Research Cross Platform nature of child_process vs. other options
• [ ] Find a way to make browser selection/getting browser file path easier
• [x] Implement solution in this branch
• [ ] Test on Win, Mac and Linux

[AvinZarlez]

Updated, going back to open package. In theory, the package plus passing in the browser should work cross platform. I'd like to at least test this on a Mac before merging into the main branch though.

@rostok Thoughts? Work on your machine as well? Documentation clear?