search

OpenPrinting / system-config-printer

Graphical user interface for CUPS administration
GNU General Public License v2.0
157 stars 88 forks source link

Make the matching rule of printer device path more flexible #183

Closed hugh712 closed 3 years ago

hugh712 commented 3 years ago
zdohnal commented 3 years ago

Hi @hugh712 ,

I'm sorry for not getting to your issue yesterday, but I managed to look into the code on your fork and I would like to comment on it tomorrow.

Anyway, thank you for the PR!

zdohnal commented 3 years ago

Did you test the PR if it works? I tried to apply my suggestions into the code, but the print queue wasn't disabled - so I'm trying to find out whether I made a mistake or the design needs adjusting.

hugh712 commented 3 years ago

Did you test the PR if it works? I tried to apply my suggestions into the code, but the print queue wasn't disabled - so I'm trying to find out whether I made a mistake or the design needs adjusting.

Yes, I tried it and it works well

hugh712 commented 3 years ago

Hi @hugh712 ,

I have a following comments about the code, would you mind looking into them?

Thank you in advance!

Zdenek

Hi

Hi @hugh712 ,

I have a following comments about the code, would you mind looking into them?

Thank you in advance!

Zdenek

@zdohnal, Hi, I have limited hardware access this week, will work on it once I have the hardware, many thanks :)

zdohnal commented 3 years ago

No problem.

hugh712 commented 3 years ago

@zdohnal,

Please help to review again, thanks :)

hugh712 commented 3 years ago

@zdohnal,

and please help to link this PR to the issue[0], thanks.

[0] https://github.com/OpenPrinting/system-config-printer/issues/182

zdohnal commented 3 years ago

Hi @hugh712 ,

I'm sorry I won't probably get to the review this week. Hope I'll manage at the beginning of the next.

I'm sorry for inconvenience :(

hugh712 commented 3 years ago

@zdohnal,

Wondering do you have some spare time to review this PR this week? :)

zdohnal commented 3 years ago

Hi @hugh712 ,

I'm sorry, I haven't checked and tested it yet - I will not promise any exact date from now, but I have this PR on my TODO list. I'll give a heads-up when I get to this.

I'm really sorry for inconvenience and still I'm grateful for the PR!

hugh712 commented 3 years ago

no problem :)

tillkamppeter commented 3 years ago

@zdohnal, the ENV{INTERFACE}!="7/1/4" is to not treat interfaces which are for IPP-over-USB, as they are handled separately by the ipp-usb or ippusbxd daemon. They cannot be used by the "usb" backend of CUPS (uses 7/1/1 and 7/1/2) or the "hp" backend of HPLIP (uses 7/1/3).

tillkamppeter commented 3 years ago

@hugh712, thanks for your contribution, I have merged your pull request now. If anyone finds corener cases which are not covered, please post an issue.

hugh712 commented 3 years ago

Great!, thank you so much :)

zdohnal commented 3 years ago

Thank you for merging the PR, Till!

@hugh712 thanks for the PR! I was able to test it at last and it works.

I'll do some minor changes regarding the code (spaces, use strstr directly...), but it looks good overall.

Again, sorry for the delay.