Skip to content

Feature to preserve inline comment spacing#129

Merged
davidvarga merged 1 commit intodavidvarga:masterfrom
Jargendas:feat/preserve-inline-comments
Apr 17, 2025
Merged

Feature to preserve inline comment spacing#129
davidvarga merged 1 commit intodavidvarga:masterfrom
Jargendas:feat/preserve-inline-comments

Conversation

@Jargendas
Copy link
Contributor

@Jargendas Jargendas commented Nov 1, 2024

In some cases (for me at least) it makes sense to preserve the spacing for inline comments. Consider the following snippet for example:

properties (Dependent, Access=public)
    startFrequency          %> [double] Start Frequency in Hz
    stopFrequency           %> [double] Stop Frequency in Hz
    centerFrequency         %> [double] Center Frequency in Hz
    resolutionBandwidth     %> [double] Resolution Bandwidth in Hz    
    sweepMode               %> [string] Sweep Mode
    refLevOffset            %> [double] Scale the amplitude of reference level in dBm
    nSamples                %> [double] Number of Samplingpoints 
    dBperDiv                %> [double] Scale the amplitude per Division of Y-axis in dB
    attenuationLev          %> [double] Adjust the input attenation in dB
    traceActive             %> [double] Indicates active Trace   
    traceState              %> [string] State of active Trace
end

The formatting is nice and tidy, however after applying the normal formatFile to it, it becomes this, which is much less readable imo:

properties (Dependent, Access=public)
    startFrequency %> [double] Start Frequency in Hz
    stopFrequency %> [double] Stop Frequency in Hz
    centerFrequency %> [double] Center Frequency in Hz
    resolutionBandwidth %> [double] Resolution Bandwidth in Hz
    sweepMode %> [string] Sweep Mode
    refLevOffset %> [double] Scale the amplitude of reference level in dBm
    nSamples %> [double] Number of Samplingpoints
    dBperDiv %> [double] Scale the amplitude per Division of Y-axis in dB
    attenuationLev %> [double] Adjust the input attenation in dB
    traceActive %> [double] Indicates active Trace
    traceState %> [string] State of active Trace
end

Therefore, I added a new special rule PreserveInlineCommentSpacing to keep the original spacing in these cases.

The downside of the solution is that the type of spacing (tabs, spaces) used by the user will also be preserved. I don't really see an easy way to change that, as tabs can represent a variable number of spaces.

@Jargendas
Copy link
Contributor Author

Okay, I did actually find a way of replacing tabs with spaces correctly, but it is quite a beast...

if numel(commentSpacing) > 0 && actComment ~= "" && obj.Configuration.specialRule('PreserveInlineCommentSpacing').ValueAsDouble() ~= 0
    spacing = commentSpacing{1};
    if obj.Configuration.specialRule('IndentationCharacter').Value == 'white-space'
        tabs = regexp(spacing, '\t');
        addedChars = 0;
        for tabIdx = tabs
            numSpaces = obj.Configuration.specialRule('IndentationCount').ValueAsDouble() - mod(numel(actCode) + addedChars + tabIdx -1, obj.Configuration.specialRule('IndentationCount').ValueAsDouble());
            leadSpacing = '';
            trailSpacing = '';
            if (tabIdx+addedChars > 1)
                leadSpacing = spacing(1:tabIdx+addedChars-1);
            end
            if (tabIdx+addedChars < length(spacing))
                trailSpacing = spacing(tabIdx+addedChars+1:end);
            end
            spacing = [leadSpacing, repelem(' ', numSpaces), trailSpacing];
            addedChars = addedChars + numSpaces - 1;
        end
    end
    actComment = [spacing, actComment];
end

@peter-gerlagh-kyos
Copy link

I have the same issue. Would love to see this merged.

@davidvarga davidvarga merged commit 06c676a into davidvarga:master Apr 17, 2025
@davidvarga
Copy link
Owner

Thanks for the PR, merged it.
"but it is quite a beast..." 👍 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants