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

Merged
merged 7 commits into from Jun 26, 2017

Conversation

Projects
None yet
3 participants
@bep
Member

bep commented Jun 25, 2017

This is pretty cool.

Fixes #3643

@bep bep added the InProgress label Jun 25, 2017

bep added some commits Jun 26, 2017

@bep bep changed the title from WIP Support open "current content page" in browser to Support open "current content page" in browser Jun 26, 2017

@bep bep added NeedsReview and removed InProgress labels Jun 26, 2017

@bep bep requested review from anthonyfok and digitalcraftsman Jun 26, 2017

@bep bep referenced this pull request Jun 26, 2017

Open

Document navigateToChanged #21

Show outdated Hide outdated commands/hugo.go
if navigate {
// It is probably more confusing than usueful

This comment has been minimized.

@anthonyfok

anthonyfok Jun 26, 2017

Contributor

Please fix minor typo: usuefuluseful

@anthonyfok

anthonyfok Jun 26, 2017

Contributor

Please fix minor typo: usuefuluseful

bep and others added some commits Jun 26, 2017

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jun 26, 2017

Contributor

This is pretty cool.

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!)

Contributor

anthonyfok commented Jun 26, 2017

This is pretty cool.

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!)

@anthonyfok

anthonyfok approved these changes Jun 26, 2017 edited

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!

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jun 26, 2017

Member

but it almost seems like it jumps to the current section! (No, wait, I am just imagining things... Hoho!)

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.

Member

bep commented Jun 26, 2017

but it almost seems like it jumps to the current section! (No, wait, I am just imagining things... Hoho!)

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.

@bep bep merged commit c825a73 into gohugoio:master Jun 26, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@@ -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")

This comment has been minimized.

@digitalcraftsman

digitalcraftsman Jun 26, 2017

Member

What about "navigate to changed content file in browser on live reload" instead?

@digitalcraftsman

digitalcraftsman Jun 26, 2017

Member

What about "navigate to changed content file in browser on live reload" instead?

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Jun 26, 2017

Member

This feature is really nice and might have been a timer saver for @rdwatters during the rewrite of the docs 😉 .

Thanks for testing it! I need some Windows user to take it for a spin, but I will ask in the forum.

I tried it too and it worked perfectly. But I'm on Linux as well.

Member

digitalcraftsman commented Jun 26, 2017

This feature is really nice and might have been a timer saver for @rdwatters during the rewrite of the docs 😉 .

Thanks for testing it! I need some Windows user to take it for a spin, but I will ask in the forum.

I tried it too and it worked perfectly. But I'm on Linux as well.

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jun 26, 2017

Contributor

I need some Windows user to take it for a spin, but I will ask in the forum.

I could reboot into Windows 10 and do some testing there too. Hope I can post my result within half an hour? :-)

Contributor

anthonyfok commented Jun 26, 2017

I need some Windows user to take it for a spin, but I will ask in the forum.

I could reboot into Windows 10 and do some testing there too. Hope I can post my result within half an hour? :-)

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jun 26, 2017

Member

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).

Member

bep commented Jun 26, 2017

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).

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jun 26, 2017

Contributor

Preliminary result under Windows 10:

  • Using Notepad as the editor, it works with both Internet Explorer 11, Microsoft Edge, and Google Chrome. Hurray!
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": WRITE
INFO 2017/06/26 15:00:11 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md

filename = C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
GetContentPage(): rel = release-notes\release-notes.md
GetContentPage(): pos = 114
  • Using Vim (from Git Bash, MINGW64) as the editor: No go... for some unknown reason, it generates a WRITE event on the directory instead of the file:
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes": WRITE
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": RENAME
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": CREATE
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": WRITE
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md

filename = C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes
GetContentPage(): rel = release-notes
GetContentPage(): pos = -1
Contributor

anthonyfok commented Jun 26, 2017

Preliminary result under Windows 10:

  • Using Notepad as the editor, it works with both Internet Explorer 11, Microsoft Edge, and Google Chrome. Hurray!
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": WRITE
INFO 2017/06/26 15:00:11 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md

filename = C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
GetContentPage(): rel = release-notes\release-notes.md
GetContentPage(): pos = 114
  • Using Vim (from Git Bash, MINGW64) as the editor: No go... for some unknown reason, it generates a WRITE event on the directory instead of the file:
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes": WRITE
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": RENAME
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": CREATE
Source changed "C:\\Users\\Anthony\\go\\src\\github.com\\gohugoio\\hugo\\docs\\content\\release-notes\\release-notes.md": WRITE
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md
INFO 2017/06/26 15:04:58 rereading C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes\release-notes.md

filename = C:\Users\Anthony\go\src\github.com\gohugoio\hugo\docs\content\release-notes
GetContentPage(): rel = release-notes
GetContentPage(): pos = -1
@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jun 26, 2017

Member

Using Vim (from Git Bash, MINGW64) as the editor: No go... for some unknown reason, it generates a WRITE event on the directory instead of the file:

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.

Member

bep commented Jun 26, 2017

Using Vim (from Git Bash, MINGW64) as the editor: No go... for some unknown reason, it generates a WRITE event on the directory instead of the file:

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.

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jun 26, 2017

Contributor

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!

Contributor

anthonyfok commented Jun 26, 2017

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!

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jun 26, 2017

Member

) I will create a PR too as an exercise for myself

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.

Member

bep commented Jun 26, 2017

) I will create a PR too as an exercise for myself

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.

anthonyfok added a commit to anthonyfok/hugo that referenced this pull request Jun 27, 2017

Make `--navigateToChanged` more robust on Windows
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 #3645

bep added a commit that referenced this pull request Jun 27, 2017

Make `--navigateToChanged` more robust on Windows
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 #3645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment