Johannes Sixt <j6t@xxxxxxxx> writes:
> Am 14.03.22 um 18:20 schrieb Glen Choo:
>> xing zhi jiang <a97410985new@xxxxxxxxx> writes:
>>> diff --git a/t/t4018/javascript-exports-function b/t/t4018/javascript-exports-function
>>> new file mode 100644
>>> index 0000000000..63b79f5991
>>> --- /dev/null
>>> +++ b/t/t4018/javascript-exports-function
>>> @@ -0,0 +1,4 @@
>>> +exports.RIGHT = function(document) {
>>> +
>>> + return ChangeMe; >>> +}
>>
>> I don't think we should include 'exports.foo = bar'. To my knowledge,
>> this is _not_ a standard ES feature, but rather the CommonJS module
>> system popularized by Node.js [1] and other frameworks. To confirm this,
>> I searched the ES spec and did not find any reference to exports.* [2].
>>
>> Even if we wanted to support nonstandard 'language features' (and this
>> label is tenuous at best, CommonJS is not trying to replace the ES
>> modules standard AFAIK), Node.js is also starting to support ES modules,
>> so I don't think this is a good long term direction for Git.
>
> It is not a priority to model hunk header regular expressions after some
> standard and to ignore stuff that is outside the standard. The goal is
> to make them useful in a majority of cases. If there exists a noticable
> chunk of code that uses non-standard constructs, then that is worth
> being supported.
Interesting, I'll take note. I'm still personally not keen on supporting
CommonJS-only patterns when we are purportedly trying to show diffs for
JavaScript, but if we think this fits the style, I'm happy to oblige.
So the question becomes "Is there a significant amount of code that uses
this pattern?" Probably - this is a fairly common pattern in Node.js
after all. But in my experience,
module.exports.RIGHT = function(document) {
return ChangeMe;
}
is even more common. The difference between 'module.exports' and
'exports' isn't worth going into (StackOverflow has all the answers, for
the curious), but if we're taking the approach of supporting CommonJS,
I'd like to be consistent and also support 'module.exports', i.e.
perhaps change:
"^(exports\\.[$_[:alpha:]][$_[:alnum:]]*[\t ]*=[\t ]*(\\(.*\\)|[$_[:alpha:]][$_[:alnum:]]*)[\t ]*=>.*)\n"
to something like:
"^((module.)?exports\\.[$_[:alpha:]][$_[:alnum:]]*[\t ]*=[\t ]*(\\(.*\\)|[$_[:alpha:]][$_[:alnum:]]*)[\t ]*=>.*)\n"
