[vim/vim] Make vim compile without the linebreak option (PR #19068)
Matthias
When compiling with all features except for linebreak, there were some
compiler errors. By slightly modifying some preprocessor conditions,
compiling without the linebreak feature should work as expected.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19068
Commit Summary
- 71cb54d Make vim compile without the linebreak option
File Changes
(4 files)
- M src/charset.c (4)
- M src/drawline.c (4)
- M src/optionstr.c (2)
- M src/structs.h (3)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr pushed 1 commit.
- 86a11c3 Readd accidentally deleted comment
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
This doesn't look right.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In addition to my suggested actions, I think making the following changes will solve the problem.
diff --git a/src/charset.c b/src/charset.c index 829ad3720..7ca8398be 100644 --- a/src/charset.c +++ b/src/charset.c @@ -1265,6 +1265,7 @@ win_lbr_chartabsize( #if defined(FEAT_LINEBREAK) || defined(FEAT_PROP_POPUP) int has_lcs_eol = wp->w_p_list && wp->w_lcs_chars.eol != NUL; +# ifdef FEAT_LINEBREAK /* * First get the normal size, without 'linebreak' or text properties */ @@ -1275,7 +1276,7 @@ win_lbr_chartabsize( // for a NUL character in the text. size = has_lcs_eol ? 1 : 0; } -# ifdef FEAT_LINEBREAK + int is_doublewidth = has_mbyte && size == 2 && MB_BYTE2LEN(*s) > 1; # endif @@ -1338,6 +1339,7 @@ win_lbr_chartabsize( else size += cells; cts->cts_start_incl = tp->tp_flags & TP_FLAG_START_INCL; +# ifdef FEAT_LINEBREAK if (*s == TAB) { // tab size changes because of the inserted text @@ -1345,10 +1347,13 @@ win_lbr_chartabsize( tab_size = win_chartabsize(wp, s, vcol + size); size += tab_size; } +# endif +# ifdef FEAT_PROP_POPUP if (tp->tp_col == MAXCOL && (tp->tp_flags & (TP_FLAG_ALIGN_ABOVE | TP_FLAG_ALIGN_BELOW))) // count extra line for property above/below ++cts->cts_prop_lines; +# endif } } if (tp->tp_col != MAXCOL && tp->tp_col - 1 > col) diff --git a/src/sign.c b/src/sign.c index 550bb25af..f3a75d472 100644 --- a/src/sign.c +++ b/src/sign.c @@ -1159,6 +1159,7 @@ sign_list_by_name(char_u *name) static void may_force_numberwidth_recompute(buf_T *buf, int unplace) { +# if defined(FEAT_LINEBREAK) tabpage_T *tp = NULL; win_T *wp = NULL; @@ -1169,6 +1170,7 @@ may_force_numberwidth_recompute(buf_T *buf, int unplace) (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u')) wp->w_nrwidth_line_count = 0; } +# endif } /*
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
I don't think it's a good idea to remove this ifdef.
In src/charset.c:
> @@ -773,7 +773,7 @@ chartabsize(char_u *p, colnr_T col)
RET_WIN_BUF_CHARTABSIZE(curwin, curbuf, p, col)
}
-#if defined(FEAT_LINEBREAK)
+#if defined(FEAT_LINEBREAK) || defined(FEAT_PROP_POPUP)
I don't think this change is necessary.
In src/drawline.c:
> @@ -2775,7 +2775,9 @@ win_line( } wlv.extra_for_textprop = FALSE; +#ifdef FEAT_LINEBREAK
Indent nested directives.
⬇️ Suggested change-#ifdef FEAT_LINEBREAK +# ifdef FEAT_LINEBREAK
In src/drawline.c:
> in_linebreak = FALSE; +#endif⬇️ Suggested change
-#endif +# endif
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
Indent nested directives.
⬇️ Suggested change-#if defined(FEAT_LINEBREAK) +# if defined(FEAT_LINEBREAK)
In src/optionstr.c:
> // When changing the 'signcolumn' to or from 'number', recompute the
// width of the number column if 'number' or 'relativenumber' is set.
if (((*args->os_oldval.string == 'n' && args->os_oldval.string[1] == 'u')
|| (*curwin->w_p_scl == 'n' && *(curwin->w_p_scl + 1) =='u'))
&& (curwin->w_p_nu || curwin->w_p_rnu))
curwin->w_nrwidth_line_count = 0;
+#endif
⬇️ Suggested change
-#endif +# endif
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
why is this change necessary?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
The problem here is that there is an inconsistency in all the files about whether to put a space here or not. I tried to use the style that the lines close to the change also use.
This is also why once I use #if defined(...) and another time just #ifdef ....
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
@chrisbra Who or what are you asking? me?
The problem here is that there is an inconsistency in all the files about whether to put a space here or not. I tried to use the style that the lines close to the change also use.
No.
If directives are nested, "basically" a space (indentation) according to the nesting level is inserted after the #.
There are a few files where the indentation of directives is incorrect, but we hope that this will be improved in the future.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
Oh, I did not know that. Thanks for pointing this out. Will try to fix this. Maybe we should add this to the coding style document?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
Matthias
@matrdr commented on this pull request.
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
There is the function may_force_numberwidth_recompute in sign.c that uses some of these fields and seems to be compiled into every configuration. So, I guess there is no way around removing this #ifdef.
The exception is the field w_nuw_cached. This one is really only needed with the linebreak feature. I added the #ifdef back for this one.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/charset.c:
> @@ -773,7 +773,7 @@ chartabsize(char_u *p, colnr_T col)
RET_WIN_BUF_CHARTABSIZE(curwin, curbuf, p, col)
}
-#if defined(FEAT_LINEBREAK)
+#if defined(FEAT_LINEBREAK) || defined(FEAT_PROP_POPUP)
The function that is within this #if block gets called from line 1271, which is within an #if defined(FEAT_LINEBREAK) || defined(FEAT_PROP_POPUP) block, so it is also called, when just FEAT_PROP_POPUP is defined.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
But this change really seems unnecessary. No idea, why I put this one in. I'll remove it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
The only thing I can find is this.
:h style-various
Sorry, there doesn't seem to be any documentation on nesting directives.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/optionstr.c:
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
Ah, it was because previously the field w_nrwidth_line_count was only compiled for the linebreak feature. Now that I enabled this field for all configurations, this can go.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
> // When changing the 'signcolumn' to or from 'number', recompute the
// width of the number column if 'number' or 'relativenumber' is set.
if (((*args->os_oldval.string == 'n' && args->os_oldval.string[1] == 'u')
|| (*curwin->w_p_scl == 'n' && *(curwin->w_p_scl + 1) =='u'))
&& (curwin->w_p_nu || curwin->w_p_rnu))
curwin->w_nrwidth_line_count = 0;
+#endif
This line was deleted, so I guess that is also a way to resolve this ;)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
> @@ -4037,12 +4037,14 @@ did_set_signcolumn(optset_T *args)
if (check_opt_strings(*varp, p_scl_values, FALSE) != OK)
return e_invalid_argument;
+#if defined(FEAT_LINEBREAK)
You are right this turned out the be unnecessary.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
Oh, I overlooked your patch. That may resolve this...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
Is this part not going back?
The order of the members has changed unexpectedly, so I recommend that you change it back.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
Good point. There is no actual reason to change the order here. I restored the original order.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/charset.c:
> if (*s == TAB)
{
// tab size changes because of the inserted text
size -= tab_size;
tab_size = win_chartabsize(wp, s, vcol + size);
size += tab_size;
}
+# endif
+# ifdef FEAT_PROP_POPUP
this can be removed, we are already inside an ifdef FEAT_PROP_POPUP
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
h_east
@h-east commented on this pull request.
In src/structs.h:
> @@ -4307,12 +4307,10 @@ struct window_S
#ifdef FEAT_GUI
scrollbar_T w_scrollbars[2]; // vert. Scrollbars for this window
#endif
-#ifdef FEAT_LINEBREAK
Please correct the code to the original.
The #ifdef ~ #endif in the member definition section should make it clear which feature the member depends on.
It should not be changed solely for the sake of ease of code modification.
https://github.com/vim/vim/blob/6625ba359e33f8223d146f3ae87a880c446b5470/src/structs.h#L4310-L4315
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr pushed 8 commits.
- 1c48041 Make vim compile without the linebreak option
- 9bb713b Readd accidentally deleted comment
- 4139fe4 Compile field of windows_S struct conditionally
- e864dbb Remove unnecessarily added #ifdef
- 0afcf80 Incorporate @h-east's suggestions
- fe08ca5 Restore original field order in struct window_S
- b1861d9 Use #ifdef instead of #if defined
- b03e680 Remove pointless #ifdef
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr commented on this pull request.
In src/charset.c:
> if (*s == TAB)
{
// tab size changes because of the inserted text
size -= tab_size;
tab_size = win_chartabsize(wp, s, vcol + size);
size += tab_size;
}
+# endif
+# ifdef FEAT_PROP_POPUP
Good catch. Fixed that.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
@matrdr pushed 1 commit.
- 4ea8f94 Revert structs.h to its original state
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Matthias
Christian Brabandt
chrisbra left a comment (vim/vim#19068)alright, thanks
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.