Skip to content

Conversation

@dhthwy
Copy link
Contributor

@dhthwy dhthwy commented Nov 11, 2025

There are definitely parts that need some refactoring, I think.

The intent was not to change any behavior or risk changing behavior, so refactoring was kept to minimum.

@dhthwy
Copy link
Contributor Author

dhthwy commented Nov 12, 2025

I tested most of this. Ought to have tests for all the commands. Manual checking sucks.

The keybinding commands work as expected with std::views::drop and std::views::reverse.

@dhthwy
Copy link
Contributor Author

dhthwy commented Nov 12, 2025

Alright, for the most part, I only did the stuff I could verify, so I guess that's it for now.

I added a couple of utility functions to miscutil.h for replacing backslashes to forward slashes. Didn't use ranges there to avoid adding another include.

@dhthwy
Copy link
Contributor Author

dhthwy commented Nov 15, 2025

I'll probably close this soon so it doesn't muddy up the PR queue. Can reopen later if there's actually a push to rewrite.

I figure there is reluctance to change known working code just for the sake of modernizing pieces of it. However, there are some parts where I think it adds some clarity and may help avoid bugs in the future for anyone that has to work on it.

Usually I inquire first before PRs like this.

@ab9rf
Copy link
Member

ab9rf commented Nov 15, 2025

I'm not ignoring this PR, I just haven't gotten to it yet. You needn't worry about messing up the PR queue.

@dhthwy
Copy link
Contributor Author

dhthwy commented Nov 16, 2025

Should've separated those commits I suppose.

@ab9rf ab9rf merged commit 982a3df into DFHack:develop Nov 16, 2025
14 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.

2 participants