Added the functionality of Preserved Comments in cupsd.conf when cups…

[Ankit3002]

…ctl is used with command line arguments

[Ankit3002]

@zdohnal I made the changes as you requested. Now I 'm printing the comment whenever I read it from cupsd.conf file. I delete the useless commits of Last PR and erroneously drop the last PR. Please review this PR and do let me know if these changes are fine or not ?

[Ankit3002]

@zdohnal I made the changes as you requested. I create a private Api to read comments and lines.

1. The private api is using same line variable to read both comments and configurations.
2. I changed the comments in cupsd.conf.in as well.
3. I moved the if else block at the end as well. Linenum checking is used only to not include blank lines between multiline comments.

[Ankit3002]

@zdohnal I made the changes as you requested. 1.I create a private Api to read comments and lines. 2. The private api use same line variable to read both comments and configurations. 3. I changed the comments in cupsd.conf.in as well. 4. I moved the if else block at the end as well. Linenum checking is used only to not include blank lines between multiline comments.

@zdohnal Any update on this ?

[zdohnal]

Hi @Ankit3002 ,

I'm sorry, I was on vacation - I'll review this today or tomorrow.

[Ankit3002]

Hi @Ankit3002 ,

I'm sorry, I was on vacation - I'll review this today or tomorrow.

No issues !

[Ankit3002]

@zdohnal cupsd.conf.in file is placed in .gitignore, should I remove it from there ? as I won't be able to push the changes in cupsd.conf.in because of this

[zdohnal]

Are you sure? I don't see the file in .gitignore.

[Ankit3002]

Are you sure? I don't see the file in .gitignore.

I 'm sorry , I got confused it with cupsd.conf

[zdohnal]

@zdohnal I made the changes as you requested. I create a private Api to read comments and lines.

1. The private api is using same line variable to read both comments and configurations.

Ok, please update the comments and debug printf to match the new function - since it was copied from cupsGetConf(), the comments+debugs stayed the same.

2. I changed the comments in cupsd.conf.in as well.

cupsAdminSetServerSettings() still puts comments into cupsd.conf file, which we can't allow once we support reading comments in the functions (otherwise we would end up with duplicity and bigger and bigger cupsd.conf with every cupsctl call). Please remove the other cupsFilePuts() which add comments from the function and migrate them into sane comments in cupsd.conf.in where it is possible.

3. I moved the if else block at the end as well. Linenum checking is used only to not include blank lines between multiline comments.

I would let blank lines between comments as it is - people adds blank lines between comments for readability, and the code will be simpler.

More comments in the review in the moment.

[Ankit3002]

@zdohnal I made the changes as you requested. Please review. This is how it works:

1. I am checking whether the line i 'm reading is comment or not .
2. if it's a comment than, than i will check for inline comments, and returns the text before '#' as you asked above. ... if it's a simple comment than i simply returns it.
3. if it's not a comment , than I read the configurations according to the earlier function.
4. Also now the function would take care about blank lines as well.
5. I have also placed the function at correct alphabetical order in file.c

[zdohnal]

@zdohnal I made the changes as you requested. Please review. This is how it works:

1. I am checking whether the line i 'm reading is comment or not .

2. if it's a comment than, than i will check for inline comments, and returns the text before '#' as you asked above. ... if it's a simple comment than i simply returns it.

You can simplify it a lot - inline comments are forbidden, so we can remove them and process as cupsGetConf() did. So in general check for '#' and then iterate over the array towards the start - if there is another text before the #, remove the # and text after # comment, don't return and let the function process the text before #. In case there is no text, let the function process further - it will remove the leading whitespace and in if (buf[0]) scope you check if the first char is # - if it is - return the buf.

In the current code there lot of duplicit code, which can be simplified by the algorithm above.

3. if it's not a comment , than I read the configurations according to the earlier function.

4. Also now the function would take care about blank lines as well.

5. I have also placed the function at correct alphabetical order in file.c

There are still more cupsFilePuts() which have to be removed from function and added to cupsd.conf.in if it makes sense.

There are public holidays in my country, I'll be back on Tuesday.

[Ankit3002]

@zdohnal I made the changes as you requested. Please review. This is how it works:

1. I am checking whether the line i 'm reading is comment or not .

2. if it's a comment than, than i will check for inline comments, and returns the text before '#' as you asked above. ... if it's a simple comment than i simply returns it.

You can simplify it a lot - inline comments are forbidden, so we can remove them and process as cupsGetConf() did. So in general check for '#' and then iterate over the array towards the start - if there is another text before the #, remove the # and text after # comment, don't return and let the function process the text before #. In case there is no text, let the function process further - it will remove the leading whitespace and in if (buf[0]) scope you check if the first char is # - if it is - return the buf.

In the current code there lot of duplicit code, which can be simplified by the algorithm above.

3. if it's not a comment , than I read the configurations according to the earlier function.

4. Also now the function would take care about blank lines as well.

5. I have also placed the function at correct alphabetical order in file.c

There are still more cupsFilePuts() which have to be removed from function and added to cupsd.conf.in if it makes sense.

There are public holidays in my country, I'll be back on Tuesday.

@zdohnal I simplified the logic as you requested, Please review them when you will be back from holidays, thank you! Now,The only cupsFilePuts() in adminutil.c are the ones which are used to write missing info.

[Ankit3002]

@zdohnal I made the changes as you requested. Please review. This is how it works:

1. I am checking whether the line i 'm reading is comment or not .

2. if it's a comment than, than i will check for inline comments, and returns the text before '#' as you asked above. ... if it's a simple comment than i simply returns it.

You can simplify it a lot - inline comments are forbidden, so we can remove them and process as cupsGetConf() did. So in general check for '#' and then iterate over the array towards the start - if there is another text before the #, remove the # and text after # comment, don't return and let the function process the text before #. In case there is no text, let the function process further - it will remove the leading whitespace and in if (buf[0]) scope you check if the first char is # - if it is - return the buf. In the current code there lot of duplicit code, which can be simplified by the algorithm above.

3. if it's not a comment , than I read the configurations according to the earlier function.

4. Also now the function would take care about blank lines as well.

5. I have also placed the function at correct alphabetical order in file.c

There are still more cupsFilePuts() which have to be removed from function and added to cupsd.conf.in if it makes sense. There are public holidays in my country, I'll be back on Tuesday.

@zdohnal I simplified the logic as you requested, Please review them when you will be back from holidays, thank you! Now,The only cupsFilePuts() in adminutil.c are the ones which are used to write missing info.

@zdohnal Any update on above ?

[Ankit3002]

@zdohnal I simplified the code according to the above suggestions, Please review them. Thank you !

[Ankit3002]

@zdohnal Please review the changes that i made. Thanks!

1. Now the code is using pointer arithmetic, there is no use of index variable.
2. I removed the if-else block and simplified the code.
3. I corrected the function definition

[Ankit3002]

@zdohnal I resolved the above issues... Now code will also takes care of line like this "--#--something" since this is not an inline comment so it won't get remove. also i put back the if block , please review Thank you for the above clarification ! The code for removing inline comment look like this

    /*
     * Remove the inline comment...
     */
     if ((ptr = strchr(buf, '#')) != NULL && _cups_isspace(*(ptr-1)))
     {
        while(ptr > buf)
        {
         // Null-terminate the string after the last non-whitespace
         if(!_cups_isspace(*(ptr-1)))
         {
           *ptr = '\0';
           break;
         }
         ptr--;
        }
     }

[Ankit3002]

@zdohnal I made the changes as you requested . Please review them, The below is how we are taking care of inline comments now.

     if ((ptr = strchr(buf, '#')) != NULL)
     {
        while(ptr > buf)
        {
         // Null-terminate the string after the last non-whitespace, unless the '#' character is escaped by a backslash ('\')
         if(!_cups_isspace(ptr[-1]) && (ptr == buf || ptr[-1] != '\\'))
         {
           *ptr = '\0';
           break;
         }
         ptr--;
        }
     }

[Ankit3002]

@zdohnal I made the changes as you requested. Please review them.

[Ankit3002]

@zdohnal I made the changes as you requested. Please review them.

@zdohnal Any Update on this ?

[zdohnal]

Hi @Ankit3002 ,

have you got time to look into the PR?

[Ankit3002]

Hi @Ankit3002 ,

have you got time to look into the PR?

Yes, I will push the changes today.

[Ankit3002]

@zdohnal Please review the changes that I made ...

[Ankit3002]

@zdohnal Please review the changes that I made ...

@zdohnal Any Update on this ?

[michaelrsweet]

Definitely not for 2.4.x, maybe for 2.5.

@zdohnal Will leave this review process to you since you've been looking at this code more than me lately...