Added handling for more file fields in GetAppmenus #12
Conversation
The 'Comment' field will not be displayed as a tooltip in VM settings. Requires QubesOS/qubes-desktop-linux-common#12 references QubesOS/qubes-issues#5076
The 'Comment' field will now be displayed as a tooltip in VM settings. Requires QubesOS/qubes-desktop-linux-common#12 references QubesOS/qubes-issues#5076
qubesappmenus/__init__.py
Outdated
| if fields: | ||
| for field in fields: | ||
| if line.startswith(field + '=') or\ | ||
| line.startswith(field + ' ='): |
There was a problem hiding this comment.
Space handling around = looks wrong, especially since partition() call below doesn't do that. Better use line.split('=', 1) and strip spaces from result
qubesappmenus/__init__.py
Outdated
| else: | ||
| for result in appmenus.get_available( | ||
| vm, fields=args.fields): | ||
| print(' | '.join(result)) |
There was a problem hiding this comment.
Please make the separate one character. Multi-character field separators may lead to various corner cases (for example how to interpret a | | b).
The 'Comment' field will now be displayed as a tooltip in VM settings. Requires QubesOS/qubes-desktop-linux-common#12 references QubesOS/qubes-issues#5076
qubesappmenus/__init__.py
Outdated
| break | ||
| if fields: | ||
| for field in fields: | ||
| if line.split('=', 1)[0].strip().startswith(field): |
There was a problem hiding this comment.
Any specific reason for startswith() instead of simple comparison? It will match other fields with common prefix. If that's for handling languages (like Comment[de]), I'd suggest printing them only when explicitly requested.
qubesappmenus/__init__.py
Outdated
| for field in fields: | ||
| if line.split('=', 1)[0].strip().startswith(field): | ||
| field_values[field] = \ | ||
| line.partition(field + '=')[2].strip() |
There was a problem hiding this comment.
And here you'll drop other field anyway. And also, this method of splitting fields is inconsistent with the if - this one will fail if there is a space before = (not a problem in practice, as the code writing it doesn't put one here, but better fix it anyway).
IMO better to split it only once and save to same variables, to be sure it's handled consistently.
qubesappmenus/__init__.py
Outdated
| @@ -575,6 +584,9 @@ def appmenus_update(self, vm, force=False): | |||
| help='required pledge for --get-available') | |||
| parser.add_argument('domains', metavar='VMNAME', nargs='+', | |||
| help='VMs on which perform requested actions') | |||
| parser.add_argument('--file-field', action='append', dest='fields', | |||
| help='File field to append to output for --get-available; can be used' | |||
| ' multiple times for multiple fields') | |||
There was a problem hiding this comment.
Please add info that this option change output format too. Preferably in manual page, if there would be one... But for now a note here would do.
Adding the optional --file-field parameter. It changes to output format to '|'-separated one, and outputs not only file name and app name, but also whatever fields from the file the user has requested. Used by GUI VM settings. references QubesOS/qubes-issues#5076
A simple man page that describes all the available options with a bit more detail than --help option.
Adding the optional --file-field parameter. It changes to output format
to '|'-separated one, and outputs not only file name and app name, but
also whatever fields from the file the user has requested. Used by
GUI VM settings.
references QubesOS/qubes-issues#5076