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

Change: Convert .md to .rtf/.txt for installers #8617

Merged
merged 1 commit into from Feb 8, 2021
Merged

Conversation

orudge
Copy link
Contributor

@orudge orudge commented Jan 30, 2021

Motivation / Problem

The licence appearing in the macOS dmg and the Windows installer looks ugly.

Description

This PR converts the markdown licence and readme to RTF for Windows and macOS, or plain text for any other platform. I’m not sure if they’re used in the .deb or similar but they will be in text instead of markdown if they are.

Limitations

The readme isn’t actually used in the installer, but CPack seems to have a reference for it so I’ve included it. It can be removed if need be.

The original .md is still installed - perhaps we should install the .rtf instead?

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 30, 2021

Pretty sure calling this a "feature" is overreaching :D Not every change has to be a feature ;) Change is more appropriate, I think?

@orudge
Copy link
Contributor Author

@orudge orudge commented Jan 30, 2021

Pretty sure calling this a "feature" is overreaching :D Not every change has to be a feature ;) Change is more appropriate, I think?

Eh, yes, Change or just Fix perhaps?

@orudge orudge changed the title Feature: Convert .md to .rtf/.txt for installers Change: Convert .md to .rtf/.txt for installers Feb 1, 2021
@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Feb 2, 2021

Code-wise this looks fine. On the functionality, I am pretty much +0.

I never understood the conversion of Markdown to .txt, which just uses another way of encoding chapters etc (with ----- and =====). To me that is just all the same with a different syntax. Just .. you can render Markdown easier than that .txt format. So I personally never cared for converting it that way.

For .rtf, I guess there is a case to be made. I never open these kind of files myself, but I can understand that being a bias on my side.

Hence my +0. Personally I would rather see we only do .rtf conversions for Windows and macOS, and leave the .md for all other. But I fully accept other developers might think otherwise :) So I leave this for another to state an opinion about :)

Snippets to understand what I mean:

Markdown:

# OpenTTD

## Table of contents

- 1.0) [About](#10-about)

.txt (undefined "formatting"):

OpenTTD
=======

Table of contents
-----------------

-   1.0) [About](#10-about)

PS: I cannot believe Pandoc doesn't resolve the [text](#anchor) in a nice way .. that really disappointed me :P

@orudge
Copy link
Contributor Author

@orudge orudge commented Feb 2, 2021

The .txt I am a bit less fussed about - it would be nice if it removed or altered the link syntax somewhat. (There may be some settings that I can pass to it that do, to be fair.) I can adjust it so that pandoc is just used for RTF on Win/Mac if preferred.

@orudge
Copy link
Contributor Author

@orudge orudge commented Feb 8, 2021

PS: I cannot believe Pandoc doesn't resolve the [text](#anchor) in a nice way .. that really disappointed me :P

It looks like -t plain does do that. However, from what I can gather, none of the Linux-based CPack generators actually use the licence or readme, so I've simplified this so it will now only convert the licence, and only on Mac/Windows.

@orudge orudge merged commit 395e015 into OpenTTD:master Feb 8, 2021
8 checks passed
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.

None yet

2 participants