-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Support open "current content page" in browser #3645
Conversation
bd43d57
to
6efa7b1
Compare
commands/hugo.go
Outdated
|
||
if navigate { | ||
|
||
// It is probably more confusing than usueful |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix minor typo: usueful
→ useful
This is really, really awesome! Totally love this new feature! Thank you Bjørn! One minor detail that did get me confused though: the demo at https://twitter.com/GoHugoIO/status/879065641315119104, it Hugo not only get the web browser jumps to the current page, but it almost seems like it jumps to the current section! (No, wait, I am just imagining things... Hoho!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My Go coding skill is still at an novice level (I am serious, and not being humble), but at a quick glance, your code looks good, and it works in Google Chrome on Debian GNU/Linux, so yes, I think it is awesome!
That would be magic! But I agree, this is a feature that I really want myself, and is golden for site-wide edits (copy-edits etc.). Thanks for testing it! I need some Windows user to take it for a spin, but I will ask in the forum. |
@@ -87,6 +88,7 @@ func init() { | |||
serverCmd.Flags().BoolVarP(&serverWatch, "watch", "w", true, "watch filesystem for changes and recreate as needed") | |||
serverCmd.Flags().BoolVarP(&serverAppend, "appendPort", "", true, "append port to baseURL") | |||
serverCmd.Flags().BoolVar(&disableLiveReload, "disableLiveReload", false, "watch without enabling live browser reload on rebuild") | |||
serverCmd.Flags().BoolVar(&navigateToChanged, "navigateToChanged", false, "navigate to changed content file on live browser reload") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about "navigate to changed content file in browser on live reload" instead?
This feature is really nice and might have been a timer saver for @rdwatters during the rewrite of the docs 😉 .
I tried it too and it worked perfectly. But I'm on Linux as well. |
I could reboot into Windows 10 and do some testing there too. Hope I can post my result within half an hour? :-) |
There is no rush with the testing, I assume it "just works" and it is some time before we release this to the public. This should work, the only gotcha would be path issues on Windows. I'm on macOs, and that works fine, too, of course (in Chrome and Safari). |
Preliminary result under Windows 10:
|
But it ALSO generates one for the file. So, I should adjust my logic to pick the first file WRITE event. I will create a new PR with a fix tomorrow. Thanks for the testing. |
You're very welcome! Yes, I am looking at pickOneWritePath() now too, and was wondering about the same. :-) I will create a PR too as an exercise for myself, but if it turns out to be an inefficient or incorrect way of doing it, please feel free to ignore my PR (if I ever get it done, haha). Have a good night! |
Yea, that would be better -- so you could test it yourself. You can check if it is a file, but the easiest is maybe to pick the "longest WRITE event". We really should get some automatic server tests running, but that will have to wait for another day. |
This ensures the new "open 'current content page' in browser" works on Windows, especially with Emacs and Vim. Special thanks to @bep for coming up with the idea of the fix. See gohugoio#3645
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This is pretty cool.
Fixes #3643