kliment / Printrun

Pronterface, Pronsole, and Printcore - Pure Python 3d printing host software
GNU General Public License v3.0
2.36k stars 995 forks source link

Only auto-capitalize if first charater is lower-case #1298

Open wavexx opened 1 year ago

wavexx commented 1 year ago

Auto-capitalize commands only if the first character is both a command character and a lower-case one.

This follows the behavior as described by https://github.com/kliment/Printrun/issues/216#issuecomment-18603449.

@frhepo FIY

rockstorm101 commented 1 year ago

Hi, I'm happy with the changes and they do work as intended. However, this change might break some people's configurations which relied on pronsole being clever there. Might not break anything since probably most firmwares are already prepared to accept lower-case commands here and there. But I can't really speak for all firmwares. I would leave this kind of change for a future 2.1.x release. I'd even suggest making auto-capitalization an option, being turned off by default. Which falls into the "feature request" category more than a "fix", hence the wait for the next minor version release.

I'm open for comments/suggestions here though.

wavexx commented 1 year ago

I know I ran into the autocapitalization issue a few times myself, and every time I rediscovered the @ prefix (I mostly use "printcore", only rarely "pronsole").

It's a double edge sword, I wonder how often people type in lower case and expect the same to work in gcode later.

But without digressing, how do you propose to set such an option? Using a variable as set via "set [var] [v]"? When I run "set" via pronsole I get nothing. Are there any options yet?

FrHePo commented 1 year ago

Hi, I'm happy with the changes and they do work as intended. However, this change might break some people's configurations which relied on pronsole being clever there. Might not break anything since probably most firmwares are already prepared to accept lower-case commands here and there. But I can't really speak for all firmwares. I would leave this kind of change for a future 2.1.x release. I'd even suggest making auto-capitalization an option, being turned off by default. Which falls into the "feature request" category more than a "fix", hence the wait for the next minor version release.

I'm open for comments/suggestions here though.

I agree. It's not that urgent and allowing the user to switch on/off would be fine.

rockstorm101 commented 1 year ago

How do you propose to set such an option? Using a variable as set via "set [var] [v]"? When I run "set" via pronsole I get nothing. Are there any options yet?

Haven't thought about it much yet, but yes, a configuration variable sounds like a good idea.