Skip to content

Remove empty title tags in generated tests#2186

Merged
tlylt merged 14 commits intoMarkBind:masterfrom
lhw-1:2166-remove-empty-title
Mar 15, 2023
Merged

Remove empty title tags in generated tests#2186
tlylt merged 14 commits intoMarkBind:masterfrom
lhw-1:2166-remove-empty-title

Conversation

@lhw-1
Copy link
Contributor

@lhw-1 lhw-1 commented Feb 23, 2023

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Resolves #2166.

As pointed out in the issue thread, instead of removing the title tag altogether when empty, it would be a better practice to simply declare a default title for sites, e.g. "Default", as is currently being done for the layout property (with the default value declared here). This PR consists of a straightforward fix by implementing a default value for the title tag.

As per the discussion in the original issue, after comparing approaches taken by other static site generators, we decided to stick with the default behavior, and additionally address the original concern of removing any unnecessary empty title tags generated (as well as prevent any empty title tags from being generated in the future).

Anything you'd like to highlight/discuss:

The default value is currently set to "Default", but suggestions on a better title are needed.

(Setting the title as the name of the .md file could be a better idea)

Once the fix has been finalized, documentation (primarily the user guide) will be updated as well.

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Remove empty title tags in generated tests


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@lhw-1 lhw-1 requested a review from a team February 23, 2023 17:39
Copy link
Contributor

@yucheng11122017 yucheng11122017 left a comment

Choose a reason for hiding this comment

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

Hi @lhw-1, thank you for the work on this!
I agree with the point that you raised that maybe using the title of the file would be a better way to generate the default title. Default is urm.. not the best 😂

@lhw-1 lhw-1 changed the title Add default value for title tags Remove empty title tags in generated tests Mar 13, 2023
@lhw-1 lhw-1 requested a review from a team March 14, 2023 05:34
Copy link
Contributor

@tlylt tlylt left a comment

Choose a reason for hiding this comment

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

LGTM

@tlylt tlylt added this to the v4.1.1 milestone Mar 15, 2023
@tlylt tlylt merged commit 320a0f2 into MarkBind:master Mar 15, 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.

Remove empty title tags in generated tests

3 participants