1041789 - Efficiency + clarity improvements in pastebin script

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Ian Connolly [:ianconnolly]]

You unnecessarily iterate FILE_TYPES when the user passes args.lang.

Also, this is better as a top-level dict (with what you call 'value' as the key) rather than a list of dicts to speed up lookup in the vast majority of cases.

Comment 1

[Anhad Jai Singh (:ffledgling)]

Attachment: Bug1041789.patch

Adding fix for the aforementioned problems.
Also did some other cleanup inside the code while I was at it.

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8467640 [details] [diff] [review]
Bug1041789.patch

Review of attachment 8467640 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/mach_commands.py
@@ +262,5 @@
> +                if not lang:
> +                    extension = os.path.splitext(filename)[1][1:]
> +                    if extension:
> +                        lang = FILE_TYPES[extension]['value']
> +                        name = FILE_TYPES[extension]['name']

These will raise a KeyError if |extension not in FILE_TYPES|.

Comment 3

[Anhad Jai Singh (:ffledgling)]

Attachment: Bug1041789.patch

Sorry about the oversight, fixed.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8468544 [details] [diff] [review]
Bug1041789.patch

Review of attachment 8468544 [details] [diff] [review]:
-----------------------------------------------------------------

r+ conditional on incorporation of style nit. I trust you to land it properly.

::: tools/mach_commands.py
@@ +268,5 @@
> +                        except KeyError:
> +                            print('Could not identify filetype')
> +                        else:
> +                            name = FILE_TYPES[extension]['name']
> +                            print('Identified file as %s' % name)

This would be much easier to read if it were written as:

try:
    lang = FILE_TYPES[extension]['value']
    name = FILE_TYPES[extension]['name']
    print('Identified file as %s' % name)
except KeyError:
    print('Could not identify filetype')

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:mhentges, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Mitchell Hentges [:mhentges] 🦀]

There's no longer a FILE_TYPES object in tools/mach_commands.py