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 <var> to VAR throughout #411

Merged
merged 2 commits into from
Sep 14, 2023
Merged

Conversation

rkdarst
Copy link
Member

@rkdarst rkdarst commented Sep 12, 2023

  • As discussed in chat (but not yet agreed)
  • Ran the command sed -i -E 's/<([0-9a-zA-Z_-]+)>/\U\1/g' content/*.md but also checked each with "meld" and removed cases
    that shouldn't be changed (like git command outputs)
    • If git command hints use <var> maybe that's a reason to not
      change?
  • Also changed some cases which weren't in angle brackets yet.
  • Time taken: about 5 minutes.
  • Review
    • There's a real chocie here: is this a good idea or not? I'm not
      sure, I've done this to see how hard it would be, not because it
      should be done.
    • Advantages: perhaps more clear what should be replaced and that
      <> are not part of the syntax? Upper case easier to scan and
      see?
    • Disadvanages: unclear what is a replacement and what is not? (I
      didn't see any places that wolud be the case.) Different than
      what others use?
    • The biggest disadvantage is that HEAD is actually a literal that
      is used in git... which is ambiguous.

@rkdarst
Copy link
Member Author

rkdarst commented Sep 12, 2023

The confusion about HEAD is the biggest problem, I think. It's enough to make me really doubt this, until we either decide that's OK or compensate somehow. But I'm not sure it's worth it.

@bast
Copy link
Member

bast commented Sep 12, 2023

For me this change makes it better and easier to follow. Maybe we can avoid using HEAD? EDIT: avoid using it in commands?

@bast
Copy link
Member

bast commented Sep 12, 2023

  • it is easier to read
  • less risk to break hedgedoc if somebody copy pastes a command

- As discussed in chat (but not yet agreed)
- Ran the command `sed -i -E 's/<([0-9a-zA-Z_-]+)>/\U\1/g'
  content/*.md` but also checked each with "meld" and removed cases
  that shouldn't be changed (like git command outputs)
  - If git command hints use `<var>` maybe that's a reason to not
    change?
- Also changed some cases which weren't in angle brackets yet.
- Time taken: about 5 minutes.
- Review
  - There's a real chocie here: is this a good idea or not?  I'm not
    sure, I've done this to see how hard it would be, not because it
    should be done.
  - Advantages: perhaps more clear what should be replaced and that
    `<>` are not part of the syntax?  Upper case easier to scan and
    see?
  - Disadvanages: unclear what is a replacement and what is not?  (I
    didn't see any places that wolud be the case.)  Different than
    what others use?
  - The biggest disadvantage is that `HEAD` is actually a literal that
    is used in git... which *is* ambiguous.
@rkdarst rkdarst force-pushed the rkdarst/change-metavar-to-uppercase branch from 7fce019 to 930f40c Compare September 12, 2023 20:26
- Since HEAD looks like a variable but isn't.
@bast bast merged commit d348cfd into main Sep 14, 2023
3 checks passed
@bast
Copy link
Member

bast commented Sep 14, 2023

Thank you!

@bast bast deleted the rkdarst/change-metavar-to-uppercase branch September 14, 2023 11:50
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.

2 participants