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

*: fix typos in command names #11730

Merged
merged 7 commits into from
Dec 14, 2023
Merged

*: fix typos in command names #11730

merged 7 commits into from
Dec 14, 2023

Conversation

gutjuri
Copy link
Member

@gutjuri gutjuri commented Dec 13, 2023

Adresses #5071

This PR includes a script that detects pages in which the filename doesn't match the page title. I've run this script and detected several typos, which are fixed by this PR.

There are cases in which a filename-title mismatch is desired, which is why we cannot put this script into the CI.

I am unsure about the titles of the following pages:

@github-actions github-actions bot added page edit Changes to an existing page(s). tooling Helper tools, scripts and automated processes. labels Dec 13, 2023
@gutjuri

This comment was marked as resolved.

@github-actions github-actions bot added the translation Issues requesting translating pages from English to other languages. label Dec 13, 2023
@tldr-bot

This comment was marked as resolved.

@sebastiaanspeck

This comment was marked as resolved.

@gutjuri

This comment was marked as resolved.

@tldr-bot
Copy link

Hello! I've noticed something unusual when checking this PR:

  • The page pages.de/common/g++.md is outdated, based on number of commands.
  • The page pages.pt_BR/common/g++.md is outdated, based on number of commands.

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

Copy link
Member

@kant kant left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

The overall changes and script addition LGTM, thanks for your contribution. I have a few minor suggestions.

Co-authored-by: K.B.Dharun Krishna <[email protected]>
@tldr-bot

This comment was marked as outdated.

@gutjuri
Copy link
Member Author

gutjuri commented Dec 13, 2023

Thanks for you feedback! Could one of the reviewers be so kind as to address the issue of the pages I'm not sure about? (See OP)

@kbdharun
Copy link
Member

kbdharun commented Dec 13, 2023

Thanks for you feedback! Could one of the reviewers be so kind as to address the issue of the pages I'm not sure about? (See OP)

It seems kind of similar to the discussion at #8786 (I will probably add an entry to the tracker issue to document how to deal with name collisions.)

Addressing your initial queries:

  • nix3-.md (-> nix-.md?): We shouldn't do this renaming as the commands aren't the same and some are experimental so both the nix and nix3 ones are necessary. (feel free to checkout Nix: update and add documentation for nix subcommands #9959 (comment))
    • Nix commands documentation is always problematic because commands like nix-build and nix build have different syntax/features the first being stable and the second being experimental (If you aren't aware of Nix, basically stable commands are currently used standardized commands written in a format whereas the nix3 commands and nix commands are experimental ones which might become stable in future or get dropped or made by a different team), both the commands are widely used by the community and it's documentation is a bit hard to interpret for a non-technical audience so this is a group where having tldr cheatsheets will most help. (as Nix is a very useful declarative package manager)
  • Regarding, nix-classic IG it is fine to leave it as it is for the above reasons.
  • For qm move_disk you can rename the title and description to refer to the commonly used qm move-disk alias alone.

@github-actions github-actions bot added the new command Issues requesting creation of a new page or PRs adding a new page for a command. label Dec 14, 2023
@tldr-bot
Copy link

Hello! I've noticed something unusual when checking this PR:

  • The page pages.de/common/g++.md is outdated, based on number of commands.
  • The page pages.pt_BR/common/g++.md is outdated, based on number of commands.
  • The page pages/linux/qm-move_disk.md seems to be a copy of pages/linux/qm-resize.md (58% matching).

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

@kbdharun kbdharun merged commit 9d383ad into tldr-pages:main Dec 14, 2023
@gutjuri gutjuri deleted the wrong-fname branch December 14, 2023 13:33
@sebastiaanspeck
Copy link
Member

sebastiaanspeck commented Dec 14, 2023

Looking at the last run of the tldr-maintenance, I found out that https://github.com/tldr-pages/tldr/blob/main/pages/linux/qm-importdisk.md?plain=1 had to be renamed to qm-import-disk OR the Dutch page didn’t need a rename of file/command. Now they don’t match with each other.

@kbdharun
Copy link
Member

Looking at the last run of the tldr-maintenance, I found out that https://github.com/tldr-pages/tldr/blob/main/pages/linux/qm-importdisk.md?plain=1 had to be renamed to qm-import-disk OR the Dutch page didn’t need a rename of file/command. Now they don’t match with each other.

This is intentional as suggested in the above discussion where we separated it into two alias pages (to follow the standard alias page template), feel free to update the Dutch translation.

@sebastiaanspeck
Copy link
Member

Looking at the last run of the tldr-maintenance, I found out that https://github.com/tldr-pages/tldr/blob/main/pages/linux/qm-importdisk.md?plain=1 had to be renamed to qm-import-disk OR the Dutch page didn’t need a rename of file/command. Now they don’t match with each other.

This is intentional as suggested in the above discussion where we separated it into two alias pages (to follow the standard alias page template), feel free to update the Dutch translation.

I am talking about the import disk command. All I can see is a discussion about move disk.

@kbdharun
Copy link
Member

I am talking about the import disk command. All I can see is a discussion about move disk.

Sorry, I confused a bit. Yeah, we can rename this to match with the translations.

Comment on lines +2 to +3
# SPDX-License-Identifier: MIT

Copy link
Member

Choose a reason for hiding this comment

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

Could use a description of what the script does here :-)

Copy link
Member

Choose a reason for hiding this comment

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

Sure, I am working on the remaining entries of the tracker issue now, will do this along with adding documentation for the scripts directory.

Copy link
Member

@sebastiaanspeck sebastiaanspeck Dec 21, 2023

Choose a reason for hiding this comment

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

Could use a description of what the script does here :-)

And maybe be improved.. right now it doesn't work on MacOS (head: illegal byte count -- -4) and it reports false positives, mainly for pages with -- (acme.sh-dns.md: # acme.sh --dns).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I am reworking it to something like this (will check your suggestion too):

#!/bin/sh
# SPDX-License-Identifier: MIT
#
# This script checks consistency between the filenames and the page
# title.
#
# Usage: ./scripts/wrong-filename.sh

# Output file for recording inconsistencies
OUTPUT_FILE="filename-consistency-check-output.txt"
# Remove existing output file (if any)
rm -f "$OUTPUT_FILE"

set -e

# Iterate through all Markdown files in the 'pages' directories
for path in $(find pages* -name '*.md' -type f); do
  # Extract the expected command name from the filename
  COMMAND_NAME_FILE=$(basename "$path" | head -c-4 | tr '-' ' ' | tr '[:upper:]' '[:lower:]')

  # Extract the command name from the first line of the Markdown file
  COMMAND_NAME_PAGE=$(head -n1 "$path" | tail -c+3 | tr '-' ' ' | tr '[:upper:]' '[:lower:]')

  # Check if there is a mismatch between filename and content command names
  if [ "$COMMAND_NAME_FILE" != "$COMMAND_NAME_PAGE" ]; then
    echo "Inconsistency found in file: $path" >> "$OUTPUT_FILE"
    echo "Expected: $COMMAND_NAME_FILE" >> "$OUTPUT_FILE"
    echo "Actual  : $COMMAND_NAME_PAGE" >> "$OUTPUT_FILE"
    echo "" >> "$OUTPUT_FILE"
  fi
done

echo "Filename consistency check completed. Output written to: $OUTPUT_FILE"

I had a second thought so instead of documenting the scripts in separate files I am adding them to the existing files header.

Copy link
Member

@kbdharun kbdharun Dec 21, 2023

Choose a reason for hiding this comment

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

Seems like head -c doesn't seem to be supported on Mac, will try out something different instead (will DM you the script to test).

Copy link
Member

Choose a reason for hiding this comment

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

Done #11810. Feel free to suggest any changes.

@sbrl
Copy link
Member

sbrl commented Dec 18, 2023

Hey, thanks for the script @gutjuri! Looks useful <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page or PRs adding a new page for a command. page edit Changes to an existing page(s). tooling Helper tools, scripts and automated processes. translation Issues requesting translating pages from English to other languages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants