What do you think about keeping the current naming scheme but whitelisting a handful of query params to use instead of using them all? The timestamp adds noise and it's probably nice to see the ID of the diff in the filename, e.g. D25478.id1541.diff? Maybe just allow the the id parameter and regex that it's value is just a string of numbers.

In D25478#13957, @speck wrote:

What do you think about keeping the current naming scheme but whitelisting a handful of query params to use instead of using them all? The timestamp adds noise and it's probably nice to see the ID of the diff in the filename, e.g. D25478.id1541.diff? Maybe just allow the the id parameter and regex that it's value is just a string of numbers.

That's a good idea. I think we should keep both id and vs. Any other parameters worth keeping?

I spent only 5 minutes playing around on a diff to see what might make the url change. The diff id is probably the key one I’d be interested in keeping. I don’t know what the other params are for (even vs).

id, vs and /new/ in URL all affect content of the generated file.

1. vs is for generating a diff between diffs in revision history, instead of against base.
2. Dxxxxx/new/ is specific to a reviewer, so output is not deterministic.

Keeping id alone could cause confusion, because the output is also affected by vs.

I'm inclined to commit as is if there is a certain degree of urgency for the fix, or otherwise take the time to check all arguments that could affect output.

Whitelisting both vs and id sound good.

Ah, before landing, please consider an additional line in the comment (like the proposed one)

Maybe better to land this to help the author. The author cannot receive email notifications.

Thanks for the reminder!