-
Notifications
You must be signed in to change notification settings - Fork 192
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
issue #87 cupsfilter showing particular errors. #88
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally approve of the changes. See my comments about coding style/indentation and the use of the "_" macro for localization.
scheduler/cupsfilter.c
Outdated
@@ -221,8 +227,10 @@ main(int argc, /* I - Number of command-line args */ | |||
i ++; | |||
if (i < argc && !infile) | |||
infile = argv[i]; | |||
else | |||
usage(opt); | |||
else{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coding style: CUPS puts open braces on their own line (not K&R), uses (traditional) 8 column tabs, and two spaces of indentation, e.g.:
else
{
_cupsLanfPrintf(stderr, _("%s: Error - ..."), argv[0]);
usage(NULL);
}
scheduler/cupsfilter.c
Outdated
@@ -184,9 +184,11 @@ main(int argc, /* I - Number of command-line args */ | |||
i ++; | |||
if (i < argc) | |||
num_options = cupsParseOptions(argv[i], num_options, &options); | |||
else | |||
else{ | |||
_cupsLangPrintf(stderr,"%s: Error - expected option after \"-a\" option.",argv[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Messages need to use the _("string")
macro in order to get scanned by gettext (which we use to build the message catalog).
else | ||
usage(opt); | ||
{ | ||
_cupsLangPrintf(stderr,_("%s: Error - expected source MIME type after \"-i\" option."),argv[0]); | ||
usage(NULL); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't exactly know why its showing like this in github. I am using vscode editor for editing files. else
statement and opening brace {
are being shown in vertical line there.
@michaelrsweet I am sorry for the mess I created. By the way is there still a mistake in indentation. Like I already told you its being shown differently in github, in vscodes it's looking like this:= If you want you can close this pull request, I will make another one in which I will make all these change in a single commit to sort all this. |
@surajkulriya It's OK, take your time. The final merge will be clean when it happens. FWIW, I was hoping VS Code might be a common editor to use across platforms but I've never been able to get the tab settings to work the way I want. For Linux, GNOME's editor is OK for simple stuff, and for macOS I've been using BBEdit for years in addition to Xcode (whose editor only seems to get worse with every new release...) |
@michaelrsweet I found this. |
@michaelrsweet I have made all the changes you had requested. Please do tell me if I am missing out something or if you want me to make some more changes as well. |
@surajkulriya Sorry, I've been busy with PWG meetings and am just getting some time to look at this again. You changes look good and I'll fix any indentation problems (if any) once I've merged. Thanks! |
Update message catalogs for new messages. Update changelog.
Thanks @michaelrsweet |
issue 87