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

Replace debug() with flog #6511

Merged
merged 19 commits into from
Jan 26, 2020
Merged

Replace debug() with flog #6511

merged 19 commits into from
Jan 26, 2020

Conversation

faho
Copy link
Member

@faho faho commented Jan 19, 2020

Description

This turns all debug() calls into FLOG or FLOGF as needed.

In doing so it increases the number of flog categories from 17 to 29.

It doesn't yet remove the debug code, in particular both fish and fish_indent (which has one total warning) read numbers for --debug, but then don't print anything, and the debug macro and implementation still exist.

Very few debug messages have been removed without replacement, these are ones I judged to be of no use, especially if they were at debug level > 2, because I recall that being super spammy. If we ever need them we can add them back.

I would like input regarding the categories and their naming. This is a user interface, and naming categories in a way that users have an idea which they should enable is good, even if the users end up being just the fish-shell members.

Also: Should we add this for 3.1 still, or wait for 3.2?

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.md

Replace with FLOGF.
This is meant to show messages related to what a terminal supports.

In particular which color or emoji it can handle.
This used debug level 5, which means it was basically unusable.
One for usable messages, another for the chatty ones.

Use like `--debug=parse-productions'*'`.
These were level 5, so I'd bet nobody ever saw them
Useful to figure out if a flog category is enabled.

We only use it in one place, but it seems like the sort of thing that
should exist.
For both input and reader, because the "reader" term is more general
and we don't have enough messages to justify multiple categories
These are related to *signal handlers* in fish_key_reader, and I don't
think this code needed to be touched since it was added.
I'm not *super*-happy with this, because pgroups and terminal
ownership and such are quite entertwined.

But hey, if all fails just use `proc'*'`
By changing to flog I inadvertently changed the warning text from "<W>
fish:" to "warning:".

Since that's also okay, let's leave it.
@zanchey
Copy link
Member

zanchey commented Jan 20, 2020

Also: Should we add this for 3.1 still, or wait for 3.2?

Please wait!

@zanchey zanchey added this to the fish 3.2.0 milestone Jan 20, 2020
@faho
Copy link
Member Author

faho commented Jan 20, 2020

Just checking if you're paying attention 😈

@faho
Copy link
Member Author

faho commented Jan 26, 2020

Okay, let's merge this now (to master, I'm not a monster!) because it's very likely to go stale fast.

@faho faho merged commit 3bb15de into fish-shell:master Jan 26, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 25, 2020
@faho faho deleted the more-flog branch September 23, 2021 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants