14 ms·

Looks like a case of print() debugging making it into production: https://github.com/gnachman/iTerm2/commit/63ec2bb0b95078a97abbb94cf28c4a42d1b67f23 https://gi

• mulhoon 2y ago
  It’s been around for 3 years?
  • CameronBanga 2y ago
    About six months. File was originally authored a few years back, but looks like this slipped in here: https://gitlab.com/gnachman/iterm2/-/commit/5db0f74bf647f6d53ea33d2491f9b8e4972c2e03 https://gitlab.com/gnachman/iterm2/-/commit/5db0f74bf647f6d5...
  • Kwpolska 2y ago
    Disabled by default until 7 months ago.
    • urronglol 2y ago
      [flagged]
      • mtmail 2y ago
        9 month ago. I don't see a connection.
• Waterluvian 2y ago
  In typescript dev I made “console.log” a linting error that cannot be merged. The occasional legitimate need uses console.info I think print debugging is fine. It has a time and place. But ideally find a way to protect yourself from accidentally leaving it in. It’s such an easy mistake to make.
  • mort96 2y ago
    Well this is an instance of writing to a file, so the JS equivalent would be calling .write on a file stream. Not exactly comparable, it wasn't literally a debug print() that someone had forgotten to delete but a log file that was enabled when verbose logging was enabled.
  • theamk 2y ago
    in similar situations, instead of saying "VERBOSE=1", I say "VERBOSE=getenv("MY_NAME_MY_APP_VERBOSE") == '1'", and set this env variable in my terminal when needed. This way there is zero chance I commit verbose-enabled debug code.
  • orliesaurus 2y ago
    not a typescript dev here: what do you use instead of console.log for debugging?
    • darknavi 2y ago
      It sounds like they use console.log, but they don't let it ship in their production code.
    • electromech 2y ago
      I liked tslog last time I tried it.
    • Waterluvian 2y ago
      console.log is reserved for devtime debugging and is a linting error so it cannot be left in. console.info is used in the occasional time we actually want to write things to console (along with console.warn, console.error, console.group, console.table, etc.). It's all it took. Just making a clear distinction between the two and communicating it with the team.
• mort96 2y ago
  That's not unreasonable code in itself, it's writing to the file only if verbose mode is enabled. This is the commit which disabled verbose mode, just before the code which removed verbose framer logging entirely: https://github.com/gnachman/iTerm2/commit/014ba7ec40fc790f650ad73ae7ff9ca7a6964aed https://github.com/gnachman/iTerm2/commit/014ba7ec40fc790f65... This is the commit which enabled VERBOSE mode: https://github.com/gnachman/iTerm2/commit/5db0f74bf647f6d53ea33d2491f9b8e4972c2e03 https://github.com/gnachman/iTerm2/commit/5db0f74bf647f6d53e... (from Jul 3, 2024) That is probably just from having set VERBOSE=1 while implementing or debugging something and forgetting to revert it to VERBOSE=0 before committing.
  • lupire 2y ago
    This is why all "temporary" settings go in .gitignored files or equivalent, or on no-merge testing branches, and we have test on the release branch that asserts for each config value that it is set to a safe value, and we have a test that reflectively asserts that every config key has a value assertion test.
    • hnlmorg 2y ago
      Or non-default compile time flags. That’s how I handle verbose debugging in my terminal emulator.
  • CGamesPlay 2y ago
    This variable might have been better named LOG_ALL_STDIO_TO_TMP. Would have made it more obvious during self-review that it shouldn't be shipped.
    • bobbylarrybobby 2y ago
      Or even better: # TODO: set back to 0 This appears as a warning in my editor, and of course is greppable
      • darknavi 2y ago