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

Issues due to removal of ArrayWriter #764

Closed
haquocviet opened this issue Jun 7, 2018 · 5 comments
Closed

Issues due to removal of ArrayWriter #764

haquocviet opened this issue Jun 7, 2018 · 5 comments

Comments

@haquocviet
Copy link

haquocviet commented Jun 7, 2018

With fmt-4.x, I often append multiple formatted stuffs to a fixed buffer in a quite neat way like this:

char buff[1024];
// I know declare like buff[1024]{} or something like that can eliminate appending zero to the end.

fmt::ArrayWriter  fmter(buff);
fmter.Write("{}, ", 42);

if (condition1) {
	fmter.Write("{}, ", .42);
}
if (condition2) {
	fmter.Write("{}, ", "string-blah");
}

//	this is convenient function to append zero to the end
fmter.c_str();

//	buff contains: "42, .42, string-blah, ";

With fmt-5.0, ArrayWriter and MemoryWriter are somehow replaced by format_to_n().
Though with a single format, the new version looks better then previous one,
it is quite clumsy compare to the previous version in case someone needs to format multiple times with different things.

This is what I have tried to do the same with the new version:

char buff[1024];
size_t fmtsize = 0;
auto result = fmt::format_to_n(buff, std::size(buff) - 1 - fmtsize, "{}, ", 42);
fmtsize += result.size;

if (condition1) {
	result = fmt::format_to_n(result.out, std::size(buff) - 1 - fmtsize, "{}, ", .42);
	fmtsize += result.size;
}

if (condition2) {
	result = fmt::format_to_n(result.out, std::size(buff) - 1 - fmtsize, "{}, ", "string-blah");
	fmtsize += result.size;
}

//	truncates trash 
buff[fmtsize] = 0;

//	buff contains: "42, .42, string-blah, ";

I wonder if there is another better way to make it neater?.
Thanks.

Update:
To avoid opening a new issue, I would append more thing here.
I realize that the fmt::format_to_n() does not support wchar_t also.

MSVS 2017 15.7 cannot compile calls below:

wchar_t buff[32];
auto res1 = fmt::format_to_n(buff, std::size(buff) - 1, L"{}", L"Wchar");

// even this fail also
auto res2 = fmt::format_to_n(buff, std::size(buff) - 1, "{}", L"Wchar");
@vitaut
Copy link
Contributor

vitaut commented Jun 8, 2018

I wonder if there is another better way to make it neater?

ArrayWriter and MemoryWriter where replaced by a more general iterator-based API. To make your use case simpler you'll need an appropriate iterator type, for example, fixed_iterator type below:

#include "fmt/format.h"

template <typename OutputIt>
class fixed_iterator {
 private:
  typedef std::iterator_traits<OutputIt> traits;

  OutputIt out_;
  std::size_t limit_;
  std::size_t count_;

 public:
  typedef std::output_iterator_tag iterator_category;
  typedef typename traits::value_type value_type;
  typedef typename traits::difference_type difference_type;
  typedef typename traits::pointer pointer;
  typedef typename traits::reference reference;

  fixed_iterator(OutputIt out, std::size_t limit)
    : out_(out), limit_(limit), count_(0) {}

  OutputIt base() const { return out_; }
  std::size_t count() const { return count_; }

  fixed_iterator& operator++() {
    if (count_++ < limit_)
      ++out_;
    return *this;
  }

  fixed_iterator operator++(int) {
    auto it = *this;
    ++*this;
    return it;
  }

  reference operator*() const {
    if (count_ >= limit_)
      throw std::runtime_error("end of buffer");
    return *out_;
  }
};

int main() {
  char buff[1024];
  auto it = fmt::format_to(fixed_iterator<char*>(buff, sizeof(buff)), "{}, ", 42);
  bool condition1 = true, condition2 = true;
  if (condition1)
    it = fmt::format_to(it, "{}, ", .42);
  if (condition2)
    it = fmt::format_to(it, "{}, ", "string-blah");

  //  truncates trash
  *it = 0;
  fmt::print("{}\n", buff);

  //  buff contains: "42, .42, string-blah, ";
}

I realize that the fmt::format_to_n() does not support wchar_t also.

This is just a matter of adding a new format_to_n overload. PRs are welcome.

@haquocviet
Copy link
Author

Thank you for your help,
Though your code helps my small issue, it should be a part of fmt instead of extending by the user (just my thought). I really want to contribute to this project but honestly C++ traits and template still challenge me.

To add more overload functions to make fmt::format_to_n() supporting wchar_t type, I have tried as below:

#include <fmt/format.h>
namespace fmt {
	template <typename OutputIt, typename... Args>
	inline OutputIt vformat_to(OutputIt out, wstring_view format_str,
		typename format_args_t<OutputIt>::type args) {
		typedef output_range<OutputIt, wchar_t> range;
		return vformat_to<arg_formatter<range>>(range(out), format_str, args);
	}

	template <typename OutputIt, typename... Args>
	inline format_to_n_result<OutputIt> format_to_n(
		OutputIt out, std::size_t n,
		wstring_view format_str,
		const Args&... args) {
		typedef internal::truncating_iterator<OutputIt> It;
		auto it = vformat_to(It(out, n), format_str,
			make_format_args<buffer_context<wchar_t>::type>(args...));
		return { it.base(), it.count() };
}

They are almost same as the version for char. Unfortunately, they do not work.
The MSVC 2017 15.7 (/std:c++latest) complains a lot the call below:

	wchar_t buff[256];
	fmt::format_to_n(buff, std::size(buff) - 1, L"{}", L"Wchar");

@vitaut
Copy link
Contributor

vitaut commented Jun 13, 2018

I added an overload of format_to_n in 0508bbc (your example was almost correct, just a small difference in context type).

Re adding something like fixed_iterator to {fmt}, I think it solves a pretty narrow use case to justify adding it to the library, but if there are more requests I'll be happy to add it.

@vitaut vitaut closed this as completed Jun 13, 2018
@haquocviet
Copy link
Author

With your new update, format_to_n() works with wchar_t like a charm. Thanks for your effort.

Now I am able to simulate two "legacy" classes: BasicArrayWriter and BasicMemoryWriter.

template <typename char_type>
class BasicArrayWriter {
private:
	char_type* ptr_;
	std::size_t size_;
	std::size_t capacity_;

public:

	BasicArrayWriter(char_type *array, std::size_t size)
		: ptr_(array), capacity_(size), size_(0) {}

	template <std::size_t SIZE>
	explicit BasicArrayWriter(char_type(&array)[SIZE])
		: ptr_(array), capacity_(SIZE), size_(0) {}

	template<typename ...Args>
	size_t write(fmt::basic_string_view<char_type> format_str, Args...args) {
		auto result = fmt::format_to_n(data() + size(), capacity() - size() - 1,
			format_str, args...);
		if (size_ + result.size < capacity_) {
			size_ += result.size;
		}
		else {
			// avoid growing buffer for zero character (c_str())
			size_ = capacity_ - 1;
		}
		return size();
	}

	template<class T>
	BasicArrayWriter<char_type>& operator<<(const T& t)
	{
		write("{}", t);
		return *this;
	}

	auto data() { return ptr_; }
	auto size() const { return size_; }
	auto capacity() const { return capacity_; }
	void clear() FMT_NOEXCEPT { size_ = 0; }

	auto c_str() {
		ptr_[size_] = 0;
		return ptr_;
	}
};

typedef BasicArrayWriter<char> ArrayWriter;
typedef BasicArrayWriter<wchar_t> WArrayWriter;



template <typename char_type, std::size_t SIZE = 256>
class BasicMemoryWriter : public basic_memory_buffer<char_type, SIZE> {
public:

	template<typename ...Args>
	inline size_t write(fmt::basic_string_view<char_type> format_str, const Args&...args) {
		auto itr = vformat_to(*this, format_str, make_format_args<buffer_context<char_type>::type>(args...));
		return size();
	}

	template<class T>
	BasicMemoryWriter<char_type, SIZE>& operator<<(const T& t)
	{
		write("{}", t);
		return *this;
	}

	void clear() FMT_NOEXCEPT { resize(0); }

	auto c_str() {
		push_back((char_type)0);
		resize(size() - 1);// keep the size as it is.
		return data();
	}

	auto str() { return c_str(); }
};


typedef BasicMemoryWriter<char> MemoryWriter;
typedef BasicMemoryWriter<wchar_t> WMemoryWriter;

For some people, two classes may be redundant but for some others like me they may relief the pain in upgrading from fmt-4.x to fmt-5.0. They are not 100% compatible to the ones which were in fmt-4.x but for most of features, they are.
I think two small functions clear() and c_str() (not existing in basic_memory_buffer<>) are very useful in cases such as in the example I posted above, and obviously, BasicArrayWriter is able to replace your recommended "class fixed_iterator".

I think two classes do not bloat much, if you don't mind please add them to the project.
Thank you.

@vitaut
Copy link
Contributor

vitaut commented Jun 22, 2018

I am not sure if these are worth including in the project, but I'll point anyone interested to your code snippets in case they have troubles migrating. Thanks!

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

No branches or pull requests

2 participants