davidvarga / MBeautifier

MBeautifier is a MATLAB source code formatter, beautifier. It can be used directly in the MATLAB Editor and it is configurable.
GNU General Public License v3.0
490 stars 77 forks source link

Bug when trying to append new entries to cell array #86

Closed FENCor closed 4 years ago

FENCor commented 4 years ago

Hi, i recently experienced an issue in MBeautifier when I tried to append an entrie to a cell array:

% Minimal Example of Beautifier Error
% In this case MBeautifier destroys the script.

TestCell = {'Text1'};
TestCell = [TestCell {'Text2'}]; % The intention is to append the cell array with additional entries.

% Expected behaviour:
% TestCell = [TestCell, {'Text2'}];
%
% Actual behaviour:
% TestCell = [TestCell{'Text2'}];

After this the script does not work any more.

Best regards, Frank.

P.S.: Beside the issue MBeautifier is an excellent tool.

davidvarga commented 4 years ago

Hi,

sorry for the late response - the problem should be fixed now.

davidvarga commented 4 years ago

Sorry, I had to revert the commit(s), it casues something else to break - this issue is much trickier than I thought.

davidvarga commented 4 years ago

Fixed the problem once again using a different solution.

FENCor commented 4 years ago

Hi David, Thanks for your answer. I modified my original test script:

TestArray = 1:10;
TestArray = [TestArray(5:10) TestArray (1:4)]; 

% Expected behaviour:
% TestArray = [TestArray(5:10) TestArray (1:4)];
%
% Actual behaviour:
% TestArray = [TestArray(5:10), TestArray(1:4)]; Commit c77308e
% TestArray = [TestArray, (5:10), TestArray, (1:4)]; Commit c792941

In case TestArray(5:10) it should be an index, in the second case TestArray (1:4) it should append TestArray and another array [1 2 3 4].

So far I found a workaround for myself. Before I ran the MBeautifier I searched for regular expressions and replaced them:

DangerousSequence = regexp(OrigContent, '\[*\w*[^,]\s[{(]', 'match');
if (~isempty(DangerousSequence))
    DisarmedSequence = replace(DangerousSequence, ' ', ', ');
    ChangedContent = regexprep(OrigContent, DangerousSequence, DisarmedSequence);
end

My workaround is not without errors since it modifies case {1 2} to case, {1, 2}.

I will also have a look at your second fix.

Best regards.

FENCor commented 4 years ago

I just tried your second fix and it works great. :-)

davidvarga commented 4 years ago

@FENCor : Glad to hear :)