Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
Merged #828 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/828#event-2618623253___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
Corrected according to the review. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/828#issuecomment-52945___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
@pavlinamv pushed 1 commit. 218633033acea714edbbdb6b21a8dba8aa3d39e8 Disable marker on multiline expression error messages -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/828/files/b2260fbae4087f55615219376906d066db7341ee..218633033acea714edbbdb6b21a8dba8aa3d39e8 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
pmatilai commented on this pull request.
> @@ -110,6 +110,12 @@ typedef struct _parseState {
static void exprErr(const struct _parseState *state, const char *msg,
const char *p)
{
+const char *s = state->str;
+
+s = strchr(s,'\n');
+if (s && (*(s+1) != '\0'))
+ p = NULL;
+
While "s" is perfectly acceptable variable name in a case like this, a more
descriptive one would make the code that little bit more readable. Say, "nl" or
even "newline", after which the whole thing reads almost as human language.
*Almost* :)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/828#pullrequestreview-285450831___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
pmatilai commented on this pull request.
> @@ -110,6 +110,12 @@ typedef struct _parseState {
static void exprErr(const struct _parseState *state, const char *msg,
const char *p)
{
+const char *s = state->str;
+
+s = strchr(s,'\n');
Waste of perfectly good screen estate... Why not just
```const char *s = strchr(state->str, '\n');```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/828#pullrequestreview-285450509___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
Corrected -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/828#issuecomment-527410044___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
@pavlinamv pushed 1 commit. b2260fbae4087f55615219376906d066db7341ee Disable marker on multiline expression error messages -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/828/files/a4e4aa041761d5eb36a486d1f058989c3a864f59..b2260fbae4087f55615219376906d066db7341ee ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Rip the marker support for multiline %{expr:...} error messages (#828)
Really bothers you, huh?
These are more on the cosmetic side, but nevertheless:
- Use existing string functions (strchr() and friends) instead of manual loops
when possible
- Use short variable names for temporary local strings, such as "s" which is
used all over rpm codebase for such purposes.
- You're adding code, not ripping something out. "Disable marker on multiline
expression error messages" would be more like it (it also isn't limited to
%{expr:...})
- Multiline is typoed "mutiline" in the commit message
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/828#issuecomment-527379676___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
