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

[BUG] Editor upper-case commands do not work #3462

Closed
chiizujin opened this issue Mar 31, 2024 · 1 comment
Closed

[BUG] Editor upper-case commands do not work #3462

chiizujin opened this issue Mar 31, 2024 · 1 comment
Labels
bug An actual error or unwanted behavior. needs-triage Yet to be looked at and verified as an a real issue/feature that could be worked on

Comments

@chiizujin
Copy link
Contributor

Describe the bug

This is kind of two closely related issues in one:

Issue 1

Upper-case editor commands, such as :UU and :DD do not work and call their lower-case equivalent if it exists.

This is because user input is in lower-case by the time it reaches the command. This is done by build_matches() in the command parser:

search_string = raw_string.lower()
for cmd in cmdset:
    cmdname, raw_cmdname = cmd.match(search_string, include_prefixes=include_prefixes)
    print(cmdname, raw_cmdname)
    if cmdname:
        matches.append(create_match(cmdname, raw_string, cmd, raw_cmdname))

This lower-cased value is then stored in cmdstring by the commmand handler:

cmd.cmdstring = cmdname  # deprecated

This is then used in the command's func():

cmd = self.cmdstring
(...)
elif cmd == ":UU":

Issue 2

The unit tests for the upper-case commands do not fail even though the commands do not work in the editor. This is because the unit tests set cmdstring directly, bypassing the lower-casing done by the command parser:

self.call(
    eveditor.CmdEditorGroup(),
    ":",
    cmdstring=":UU",
    msg="Reverted all changes to the buffer back to original state.",
)

I suspect that when the editor commands were originally created they did work and the lower-casing was something that was changed later, but because the unit tests directly set cmdstring they continued to pass.

Resolution

I think it would be too messy/hacky to allow upper-case commands through and this might not work for anyone who uses a different command parser.

The logical solution to me would be to replace the upper-case editor commands with lower-case versions, such as u! and d!. This would fix the broken commands for the default installation.

This is what I have done in my own installation but I wanted to open it up for comment in case there's a better way to do it.

There is also the issue of the unit tests, which I do not know how to fix so that they will catch upper-case failures. Although now, anyone trying to add an upper-case editor command would find that it wouldn't work in-game.

@chiizujin chiizujin added bug An actual error or unwanted behavior. needs-triage Yet to be looked at and verified as an a real issue/feature that could be worked on labels Mar 31, 2024
@Griatch
Copy link
Member

Griatch commented Apr 1, 2024

Fixed in 763699e (gave the wrong issue reference) by using the Command's .raw_string instead of .cmdname property. This preserves the capitalization all the way to the command, one just does .raw_string[:len(self.cmdname)] to make sure to get only the command-name, but with the original capitalization 😁

@Griatch Griatch closed this as completed Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An actual error or unwanted behavior. needs-triage Yet to be looked at and verified as an a real issue/feature that could be worked on
Projects
None yet
Development

No branches or pull requests

2 participants