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

Shortcodes: Update readfile to throw compile error when file not found #1457

Merged
merged 3 commits into from
Apr 20, 2023
Merged

Shortcodes: Update readfile to throw compile error when file not found #1457

merged 3 commits into from
Apr 20, 2023

Conversation

aimeeu
Copy link
Contributor

@aimeeu aimeeu commented Mar 3, 2023

  • Update the readfile shortcode to throw a compile error when the file is not found.
  • Update userguide

Currently the shortcode prints a message on the UI, which presents a problem when refactoring a docs site. A UI message requires visual conformation and is easy to miss when moving or renaming directories. Throwing a compile error ensures the tech writer knows there is a problem.

With this modification, the default behavior is to throw a compile error:

readfile-compile-error

Alternately, the user can display a message on the page instead of having Hugo throw a compile error (this implements deining's suggestion)

readfile-ui-msg

Update readfile shortcode to throw a compile error when the
file is not found. Currenlty the shortcode prints a message on the
UI, which presents a problem when refactoring a docs site. A UI
message requires visual conformation and is easy to miss when moving
or renaming directories.
@aimeeu
Copy link
Contributor Author

aimeeu commented Apr 17, 2023

@LisaFC can someone review this please?

@LisaFC
Copy link
Collaborator

LisaFC commented Apr 17, 2023

Looks fine to me, but @geriom can you take a look as you wrote the original shortcode?

@LisaFC LisaFC requested a review from geriom April 17, 2023 17:24
Copy link
Collaborator

@deining deining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A UI message requires visual conformation and is easy to miss when moving or renaming directories.

I see the point, and yes, I consider this PR an improvement and agree that throwing a compile error should be made the default behaviour. However, I can imagine situations where throwing a compile error is unwanted (e.g. with draft documents). What about adding and evaluating a shortcode parameter draft? With attribute draft set to true, the current behaviour (printing out an error message in the code) would apply. This way, one could author:

{{< readfile draft=true >}}
...
{{< /readfile >}}

Without parameter draft, a compile error would be thrown.

Just my tow cents ....

layouts/shortcodes/readfile.html Outdated Show resolved Hide resolved
@deining deining self-requested a review April 18, 2023 19:26
Copy link
Collaborator

@geriom geriom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@geriom geriom merged commit 29e1dec into google:main Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants