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

Feature Request: Add write_n() function to fmt::BasicWriter that restricts output to a specified number of characters. #298

Closed
KevinDHall opened this issue Apr 1, 2016 · 10 comments

Comments

@KevinDHall
Copy link

@KevinDHall KevinDHall commented Apr 1, 2016

The idea is to make write_n() be a replacement for the snprintf() family of functions.

The reason this would be useful is to prevent fmt::BasicArrayWriter from throwing an exception when output cannot fit in the buffer. Currently one may wrap write() in a try block, but the element that caused the exception to be thrown will not be printed at all -- not even partially. Often it is desired to get partial output. One example is copying long strings (log or error messages). It is better to get some of the text instead of none of it.

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Apr 1, 2016

Thanks for the suggestion, I think this is a useful functionality worth implementing. The only thing is that it should be a separate buffer and writer, possibly a modification of FixedBuffer and ArrayWriter, rather than a new method, because this should work both with formatting and writing API.

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Apr 1, 2016

But as a heads up, it might take a while till I get to it. Maybe someone else would like to contribute though.

@KevinDHall

This comment has been minimized.

Copy link
Author

@KevinDHall KevinDHall commented Apr 1, 2016

I'd be happy to try contributing.

The challenge I saw looking at the code is that the code to do the writing uses resize() and grow_buffer() and doesn't allow for a max_count to be passed around easily.

If you are willing to give a bit more high-level direction on where you'd like a seam to be made, I'd be happy to try implementing something.

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Apr 2, 2016

I think the easiest solution is to make Buffer::grow return bool - true if the buffer has grown to the required size, false if it hasn't (unchanged capacity & size):

template <typename Char>
bool fmt::internal::FixedBuffer<Char>::grow(std::size_t) {
  return false;
}

Then comes a more complicated part of making all callers of grow check the return value and adjust the behavior accordingly, for example:

// Buffer::push_back
void push_back(const T &value) {
  if (size_ < capacity_ || grow(size_ + 1))
    ptr_[size_++] = value;
}

void Buffer<T>::append(const U *begin, const U *end) {
  std::size_t new_size = size_ + internal::to_unsigned(end - begin);
  if (new_size > capacity_ && !grow(new_size)) {
    new_size = capacity_;
    end = begin + new_size;
  }
  std::uninitialized_copy(begin, end,
                          internal::make_ptr(ptr_, capacity_) + size_);
  size_ = new_size;
}

etc.

Numeric formatters might be a bit of a pain.

@randrewy

This comment has been minimized.

Copy link

@randrewy randrewy commented Aug 30, 2017

Any progress on this? Or may be there is other alternative to snprintf-like behaviour?

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Sep 2, 2017

A simple workaround is to format into a memory writer and then truncate the result. It is suboptimal so it's on my TODO list to add direct support for truncated output.

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Jan 15, 2018

The std branch now has an experimental range support. Currently only (unbounded) output ranges are supported, but it should be relatively straightforward to extend the implementation to support fixed-size ones.

@randrewy

This comment has been minimized.

Copy link

@randrewy randrewy commented Jan 15, 2018

This is great news!

@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Mar 28, 2018

The write API is being replaced with the format API with compile-time parsing of format strings which will have a way to limit output size (format_to_n in #518). Therefore closing in favor of #518.

@vitaut vitaut closed this Mar 28, 2018
@vitaut

This comment has been minimized.

Copy link
Contributor

@vitaut vitaut commented Mar 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.