[GH-ISSUE #102] `char` to `const char` for chart #84

New issue

Closed

opened 2026-05-05 11:37:56 -06:00 by gitea-mirror · 4 comments

gitea-mirror commented

2026-05-05 11:37:56 -06:00

Owner

Originally created by @Alexhuszagh on GitHub (Mar 9, 2017).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/102

chart.h and chart.c currently use char * for 3 functions (chart_series_set_labels_num_format, chart_series_set_trendline_name, chart_axis_set_num_format), without modifying the source buffers. These should take const char * as an argument, rather than char *, both as a guarantee that they will not be modified, but also for integration with STL containers (std::string::data() and std::vector<char>::data() both return const char *).

I've attached diffs of the changes required to make this change.

chart.h

diff --git a/include/xlsxwriter/chart.h b/include/xlsxwriter/chart.h
index 42349c1..956817b 100644
--- a/include/xlsxwriter/chart.h
+++ b/include/xlsxwriter/chart.h
@@ -1768,7 +1768,7 @@ void chart_series_set_labels_percentage(lxw_chart_series *series);
  * For more information see @ref chart_labels.
  */
 void chart_series_set_labels_num_format(lxw_chart_series *series,
-                                        char *num_format);
+                                        const char *num_format);
 
 /**
  * @brief Set the font properties for chart data labels in a series
@@ -1983,7 +1983,8 @@ void chart_series_set_trendline_intercept(lxw_chart_series *series,
  *
  * For more information see @ref chart_trendlines.
  */
-void chart_series_set_trendline_name(lxw_chart_series *series, char *name);
+void chart_series_set_trendline_name(lxw_chart_series *series,
+                                     const char *name);
 
 /**
  * @brief Set the trendline line properties for a chart data series.
@@ -2279,7 +2280,7 @@ void chart_axis_set_num_font(lxw_chart_axis *axis, lxw_chart_font *font);
  * **Axis types**: This function is applicable to to all axes types.
  *                 See @ref ww_charts_axes.
  */
-void chart_axis_set_num_format(lxw_chart_axis *axis, char *num_format);
+void chart_axis_set_num_format(lxw_chart_axis *axis, const char *num_format);
 
 /**
  * @brief Set the line properties for a chart axis.

chart.c

diff --git a/src/chart.c b/src/chart.c
index 9ecfacf..1351f7b 100644
--- a/src/chart.c
+++ b/src/chart.c
@@ -5384,7 +5384,8 @@ chart_series_set_labels_percentage(lxw_chart_series *series)
  * Set an data labels number format.
  */
 void
-chart_series_set_labels_num_format(lxw_chart_series *series, char *num_format)
+chart_series_set_labels_num_format(lxw_chart_series *series,
+                                   const char *num_format)
 {
     if (!num_format)
         return;
@@ -5531,7 +5532,7 @@ chart_series_set_trendline_intercept(lxw_chart_series *series,
  * Set a line type for a series trendline.
  */
 void
-chart_series_set_trendline_name(lxw_chart_series *series, char *name)
+chart_series_set_trendline_name(lxw_chart_series *series, const char *name)
 {
     if (!name)
         return;
@@ -5687,7 +5688,7 @@ chart_axis_set_num_font(lxw_chart_axis *axis, lxw_chart_font *font)
  * Set an axis number format.
  */
 void
-chart_axis_set_num_format(lxw_chart_axis *axis, char *num_format)
+chart_axis_set_num_format(lxw_chart_axis *axis, const char *num_format)
 {
     if (!num_format)
         return;

Originally created by @Alexhuszagh on GitHub (Mar 9, 2017). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/102 [chart.h](https://github.com/jmcnamara/libxlsxwriter/blob/master/include/xlsxwriter/chart.h) and [chart.c](https://github.com/jmcnamara/libxlsxwriter/blob/master/src/chart.c) currently use `char *` for 3 functions (`chart_series_set_labels_num_format`, `chart_series_set_trendline_name`, `chart_axis_set_num_format`), without modifying the source buffers. These should take `const char *` as an argument, rather than `char *`, both as a guarantee that they will not be modified, but also for integration with STL containers (`std::string::data()` and `std::vector<char>::data()` both return `const char *`). I've attached diffs of the changes required to make this change. **chart.h** ```diff diff --git a/include/xlsxwriter/chart.h b/include/xlsxwriter/chart.h index 42349c1..956817b 100644 --- a/include/xlsxwriter/chart.h +++ b/include/xlsxwriter/chart.h @@ -1768,7 +1768,7 @@ void chart_series_set_labels_percentage(lxw_chart_series *series); * For more information see @ref chart_labels. */ void chart_series_set_labels_num_format(lxw_chart_series *series, - char *num_format); + const char *num_format); /** * @brief Set the font properties for chart data labels in a series @@ -1983,7 +1983,8 @@ void chart_series_set_trendline_intercept(lxw_chart_series *series, * * For more information see @ref chart_trendlines. */ -void chart_series_set_trendline_name(lxw_chart_series *series, char *name); +void chart_series_set_trendline_name(lxw_chart_series *series, + const char *name); /** * @brief Set the trendline line properties for a chart data series. @@ -2279,7 +2280,7 @@ void chart_axis_set_num_font(lxw_chart_axis *axis, lxw_chart_font *font); * **Axis types**: This function is applicable to to all axes types. * See @ref ww_charts_axes. */ -void chart_axis_set_num_format(lxw_chart_axis *axis, char *num_format); +void chart_axis_set_num_format(lxw_chart_axis *axis, const char *num_format); /** * @brief Set the line properties for a chart axis. ``` **chart.c** ```diff diff --git a/src/chart.c b/src/chart.c index 9ecfacf..1351f7b 100644 --- a/src/chart.c +++ b/src/chart.c @@ -5384,7 +5384,8 @@ chart_series_set_labels_percentage(lxw_chart_series *series) * Set an data labels number format. */ void -chart_series_set_labels_num_format(lxw_chart_series *series, char *num_format) +chart_series_set_labels_num_format(lxw_chart_series *series, + const char *num_format) { if (!num_format) return; @@ -5531,7 +5532,7 @@ chart_series_set_trendline_intercept(lxw_chart_series *series, * Set a line type for a series trendline. */ void -chart_series_set_trendline_name(lxw_chart_series *series, char *name) +chart_series_set_trendline_name(lxw_chart_series *series, const char *name) { if (!name) return; @@ -5687,7 +5688,7 @@ chart_axis_set_num_font(lxw_chart_axis *axis, lxw_chart_font *font) * Set an axis number format. */ void -chart_axis_set_num_format(lxw_chart_axis *axis, char *num_format) +chart_axis_set_num_format(lxw_chart_axis *axis, const char *num_format) { if (!num_format) return; ```

gitea-mirror

2026-05-05 11:37:56 -06:00

closed this issue
added the
short term
label

gitea-mirror commented

2026-05-05 11:38:01 -06:00

Author

Owner

@Alexhuszagh commented on GitHub (May 22, 2017):

I'm wondering if there is any update on this issue? I wrote C++ wrappers for your C API (that I am looking to maintain) using object-oriented design and this would be the last remaining step so I could build off the source tree.

@Alexhuszagh commented on GitHub (May 22, 2017): I'm wondering if there is any update on this issue? I wrote C++ wrappers for your C API (that I am looking to maintain) using object-oriented design and this would be the last remaining step so I could build off the source tree.

gitea-mirror commented

2026-05-05 11:38:04 -06:00

Author

Owner

@jmcnamara commented on GitHub (May 23, 2017):

I've been very busy and haven't had time to work on my OS projects recently. However, I'll try push this up soon.

It would be good if there was some test, or tests, that I could add that would catch this and other C++ issues in the interface. Can you suggest something?

@jmcnamara commented on GitHub (May 23, 2017): I've been very busy and haven't had time to work on my OS projects recently. However, I'll try push this up soon. It would be good if there was some test, or tests, that I could add that would catch this and other C++ issues in the interface. Can you suggest something?

gitea-mirror commented

2026-05-05 11:38:06 -06:00

Author

Owner

@Alexhuszagh commented on GitHub (May 23, 2017):

@jmcnamara No worries, thanks for all the help.

I can think of a few solutions (although none are preferable):

Add you as a collaborator for the project with libxlsxwriter as a submodule using Travis CI to test for any changes. (This adds another project for you to maintain, which I'd rather not do).
I add libxlsxwriter as a submodule and track specific revisions rather than master, and update the language bindings whenever the interface changes.

The bindings only wrap API functions, so this should change minimally and any changes on your end that affect the wrappers should also affect other programs, meaning this is not a unique issue.

A sample of a C++ binding declaration is below. It's mainly wrapped for OOP, and to respect C++'s RAII idiom. I was mostly using this for my own personal use, but I'm open to changing the interface to better reflect the C API (so typedef lxw_row_t Row; becomes typedef lxw_row_t row_t;, ChartSeries becomes chart_series, and namespace xlsxwriter becomes lxw).

#include <cstdint>
#include <string>
#include <xlsxwriter/common.h>


namespace xlsxwriter
{

// these typedefs are included from a separate file, and are normally not present
typedef lxw_row_t Row;
typedef lxw_col_t Column;


class ChartSeries
{
public:
    ChartSeries() = default;
    ChartSeries(const ChartSeries&) = default;
    ChartSeries & operator=(const ChartSeries&) = default;
    ChartSeries(ChartSeries &&other);
    ChartSeries & operator=(ChartSeries &&other);

    // DATA
    void set_categories(const std::string &sheetname,
        const Row first_row,
        const Column first_col,
        const Row last_row,
        const Column last_col);
// ... many lines ellipsed

private:
    lxw_chart_series *ptr = nullptr;

    friend class Chart;

    ChartSeries(lxw_chart_series *ptr);
};

}   /* xlsxwriter */

@Alexhuszagh commented on GitHub (May 23, 2017): @jmcnamara No worries, thanks for all the help. I can think of a few solutions (although none are preferable): 1. Add you as a collaborator for the project with libxlsxwriter as a submodule using Travis CI to test for any changes. (This adds another project for you to maintain, which I'd rather not do). 2. I add libxlsxwriter as a submodule and track specific revisions rather than master, and update the language bindings whenever the interface changes. The bindings only wrap API functions, so this should change minimally and any changes on your end that affect the wrappers should also affect other programs, meaning this is not a unique issue. A sample of a C++ binding declaration is below. It's mainly wrapped for OOP, and to respect C++'s RAII idiom. I was mostly using this for my own personal use, but I'm open to changing the interface to better reflect the C API (so `typedef lxw_row_t Row;` becomes `typedef lxw_row_t row_t;`, `ChartSeries` becomes `chart_series`, and `namespace xlsxwriter` becomes `lxw`). ```cpp #include <cstdint> #include <string> #include <xlsxwriter/common.h> namespace xlsxwriter { // these typedefs are included from a separate file, and are normally not present typedef lxw_row_t Row; typedef lxw_col_t Column; class ChartSeries { public: ChartSeries() = default; ChartSeries(const ChartSeries&) = default; ChartSeries & operator=(const ChartSeries&) = default; ChartSeries(ChartSeries &&other); ChartSeries & operator=(ChartSeries &&other); // DATA void set_categories(const std::string &sheetname, const Row first_row, const Column first_col, const Row last_row, const Column last_col); // ... many lines ellipsed private: lxw_chart_series *ptr = nullptr; friend class Chart; ChartSeries(lxw_chart_series *ptr); }; } /* xlsxwriter */ ```

gitea-mirror commented

2026-05-05 11:38:07 -06:00

Author

Owner

@jmcnamara commented on GitHub (Jun 25, 2017):

I've fixed this on master and added a test so that other omissions like this are caught in the future.

Thanks.

@jmcnamara commented on GitHub (Jun 25, 2017): I've fixed this on master and added a test so that other omissions like this are caught in the future. Thanks.