Skip to content

Add %q support.#793

Closed
dhylands wants to merge 1 commit into
micropython:masterfrom
dhylands:print-q
Closed

Add %q support.#793
dhylands wants to merge 1 commit into
micropython:masterfrom
dhylands:print-q

Conversation

@dhylands
Copy link
Copy Markdown
Contributor

@dhylands dhylands commented Aug 9, 2014

This allows us to use print("%q", some_qstr) instead
of print("%s", qstr_str(some_qstr))

This has a net savings of 68 bytes on the stmhal port.

This allows us to use print("%q", some_qstr) instead
of print("%s", qstr_str(some_qstr))
@dpgeorge
Copy link
Copy Markdown
Member

This is useful, but might need some consideration and discussion:

  1. If we allow %q in print (as in mp_obj_print), and not in general printf, won't that be confusing?
  2. If we allow %q in print and printf, that means we must always use our own printf routines (not the system provided ones). This might even be the way we want to go.
  3. This patch factors out vsnprintf from stmhal and puts it in py/ core. Is this what we want? If so it should be a separate PR that gets merged first.
  4. snprintf is not factored out along with vsnprintf. Why not?

@dhylands
Copy link
Copy Markdown
Contributor Author

This is useful, but might need some consideration and discussion:

If we allow %q in print (as in mp_obj_print), and not in general printf, won't that be confusing?

Perhaps.

If we allow %q in print and printf, that means we must always use our own printf routines (not the system provided ones). This might even be the way we want to go.

Personally, I think it is the way to go. The only reason we can't right now is that we use the native snprintf to print doubles:
https://github.com/micropython/micropython/blob/master/py/objfloat.c#L60
I think that's the only place in the code that we currently need to use native printf (or variants).

This patch factors out vsnprintf from stmhal and puts it in py/ core. Is this what we want? If so it should be a separate PR that gets merged first.

The emergency exception buf code uses vsnprintf:
https://github.com/micropython/micropython/blob/master/py/objexcept.c#L340

snprintf is not factored out along with vsnprintf. Why not?

I could have factored it and renamed it to mp_snprintf, but objfloat calls snprintf and it needs to be the native one.

@dpgeorge
Copy link
Copy Markdown
Member

For very constrained ports, it might be beneficial to be able use the system provided printf, so as not to duplicate this code by also having to include the uPy version.

On the other hand, we pretty much need to include our own printf (or variant thereof) to produce Python compliant output when printing.

It's pretty confusing at the moment what printf support exists, and what is needed, and what is used by who...

@pfalcon
Copy link
Copy Markdown
Contributor

pfalcon commented Aug 11, 2014

IMHO, problem is dichotomic: "system" provided stuff can be more bloated, or can be broken (see #748). But if we do dynamic linking with decent libc, it makes sense to use its functions. So, ideally we should have our own implementations for as much stuff as possible, but use it based on config options. And currently we clearly lacking on general and consistent support for "our own stuff", so would be nice to boost that.

@dpgeorge
Copy link
Copy Markdown
Member

Main issue at hand is that if we allow %q in printf strings, then we can't swap in a system provided verision.

@dhylands
Copy link
Copy Markdown
Contributor Author

Yeah - I agree. Let's hold of on this for a while.

We should probably cleanup all of the printf usage (except perhaps for debug stuff) and then make a decision.

@dpgeorge
Copy link
Copy Markdown
Member

Ok, agree. Tidy up and consolidate printf first.

@dpgeorge
Copy link
Copy Markdown
Member

Superseded by #1179.

@dpgeorge dpgeorge closed this Apr 11, 2015
@dhylands dhylands deleted the print-q branch November 8, 2015 02:35
larsks added a commit to larsks/micropython that referenced this pull request May 2, 2018
This commit replaces the literal calls to `esptool.py` with the
`$(ESPTOOL)` Makefile variable. This allows one to set the esptool
invocation on the Make command line:

    make ESPTOOL="python2 $(which esptool.py)"

(or via the environment, an include file, etc)

Closes micropython#793
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.

3 participants