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

Adds newline after 'Building Sites' message #7580

Conversation

jwarner112
Copy link
Contributor

As mentioned in #7579, the logs keep tripping me up so I made this change, and I'm hoping it helps annoy others less as well.

Going over the contributors' guidelines:

  • Sign the CLA
  • Test cases for the code -- this was so small I'm not sure if you'd make this rule apply? I manually verified it, at least
  • go fmt was run, though it made no changes
  • No docs, same reasoning as Test Cases. Let me know if you think otherwise!
  • Squashed to one commit
  • mage check finished with exit code 0
  • Git commit message guidelines followed as best as I understood them

I think that's everything but I'm happy to discuss and make changes. 🤘

@bep
Copy link
Member

bep commented Aug 20, 2020

2 comments:

  • I suspect this change will break the "hide when done" logic; whether that logic is worth keeping is worth discussing.
  • Also, have a look at the commit message guidelines (ref. the commit message title/subject)

@jwarner112
Copy link
Contributor Author

I suspect this change will break the "hide when done" logic; whether that logic is worth keeping is worth discussing.

I'm not sure what you mean about the "hide when done" logic; I've run all the tests with no failure, and haven't found it yet to test manually. Where would I find it?

@bep
Copy link
Member

bep commented Aug 21, 2020

I'm not sure what you mean about the "hide when done" logic;

The "Building sites..." message is just showing when the site is... building. It's removed from terminal when done. If you add a newline, that logic is broken.

@jwarner112
Copy link
Contributor Author

jwarner112 commented Aug 21, 2020

Ooh, okay. I guess I've got a bug to report, then -- it's never hidden itself for me, to the point that I didn't know it was even supposed to. Here's what I've always gotten:

$ hugo
Building sites … WARN 2020/08/21 00:04:19 I'm a little teapot
WARN 2020/08/21 00:04:19 short and stout

                   | EN  
-------------------+-----
  Pages            | 28  
  Paginator pages  |  1  
  Non-page files   |  0  
  Static files     |  1  
  Processed images |  2  
  Aliases          |  4  
  Sitemaps         |  1  
  Cleaned          |  0  

Total in 96 ms
$

I would say that if that behavior were fixed, my problem would go away -- however, I think it would reach and display my warnf statements, and that the first of those might get erased when the Building Sites ... clears?


edit: System info

Linux: Linux jbox 5.4.0-42-generic #46~18.04.1-Ubuntu SMP Fri Jul 10 07:21:24 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
WM: GNOME 3.28.4
Terminal: GNOME Terminal 3.28.2 using VTE 0.52.2 +GNUTLS -PCRE2

@bep
Copy link
Member

bep commented Aug 21, 2020

So, I added that clever construct, but I have seen it failing in some situations myself, so what I suggest you do as part of this is:

  1. Add a newline (like this PR)
  2. Remove the ifTerminal(hideCursor) logic (remove the function)
  3. Edit the text from "Building sites …" to ... "Start building sites …"

…d completes; Append newline to the message instead (Fixes gohugoio#7579)
@jwarner112 jwarner112 force-pushed the jwarner112-add-newline-after-BuildingSites-message branch from 0f592e7 to de050d1 Compare August 22, 2020 00:29
@jwarner112
Copy link
Contributor Author

Cool, cool! I've made those changes and I believe adequately resolved the commit message critique.

@bep bep merged commit d39636a into gohugoio:master Aug 22, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants