Skip to content

Conversation

@lathapatil
Copy link
Contributor

Fixes #2600

Comment on lines 195 to 199

IAutoEditStrategy[] autoEditStrategies= new IAutoEditStrategy[] { getAutoIndentStrategy(sourceViewer, contentType) };
ArrayList<IAutoEditStrategy> autoEditStrategiesList= new ArrayList<>(Arrays.asList(autoEditStrategies));
autoEditStrategiesList.add(new SurroundWithBracketsStrategy(sourceViewer));
return autoEditStrategiesList.toArray(new IAutoEditStrategy[0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
IAutoEditStrategy[] autoEditStrategies= new IAutoEditStrategy[] { getAutoIndentStrategy(sourceViewer, contentType) };
ArrayList<IAutoEditStrategy> autoEditStrategiesList= new ArrayList<>(Arrays.asList(autoEditStrategies));
autoEditStrategiesList.add(new SurroundWithBracketsStrategy(sourceViewer));
return autoEditStrategiesList.toArray(new IAutoEditStrategy[0]);
return new IAutoEditStrategy[] { getAutoIndentStrategy(sourceViewer, contentType), new SurroundWithBracketsStrategy(sourceViewer) };

if (bracketsMap.containsKey(command.text)) {
if (command.text != null && bracketsMap.containsKey(command.text)) {
try {
ITextSelection selection= (ITextSelection) sourceViewer.getSelectionProvider().getSelection();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as suggested by @szarnekow earlier, it'd be best to use command.getSelection() here instead of asking the viewer again.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2024

Test Results

 1 821 files  +  607   1 821 suites  +607   1h 31m 24s ⏱️ + 18m 28s
 7 729 tests ±    0   7 501 ✅ +    6  228 💤  -   5  0 ❌  - 1 
24 348 runs  +8 116  23 599 ✅ +7 883  749 💤 +234  0 ❌  - 1 

Results for commit 7a453c6. ± Comparison against base commit 8bd169c.

♻️ This comment has been updated with latest results.

@jukzi
Copy link
Contributor

jukzi commented Dec 11, 2024

locally tested: This PR fixes jdt.ui SmokeViewsTest
image

@lathapatil lathapatil force-pushed the Issues/2600_IssuesFix_introducedBy_PR_2051 branch from 272e7f8 to 7a453c6 Compare December 12, 2024 07:07
@mickaelistria
Copy link
Contributor

@szarnekow Do you agree that this PR fixes your concerns about configurability? Any objection to merge?

@BeckerWdf
Copy link
Member

Can we merge this.
This also causes issues e.g. with the templates view where autoEditStrategies in

IAutoEditStrategy[] autoEditStrategies= configuration.getAutoEditStrategies(this, t);
List<IAutoEditStrategy> autoEditStrategiesList= new ArrayList<>(Arrays.asList(autoEditStrategies));

in SourceViewer#configure is null.

@szarnekow
Copy link
Contributor

@szarnekow Do you agree that this PR fixes your concerns about configurability? Any objection to merge?

Yes, this looks all good to me.

@merks merks dismissed mickaelistria’s stale review December 13, 2024 17:15

I think asking @szarnekow his opinion implies that you approve.

@merks
Copy link
Contributor

merks commented Dec 13, 2024

Just for safety and comfort, I restarted the Linux verification. For a recent p2 PR, I found that repeated re-running eventually succeeds, so I'm interested to see if that works here too.

@merks
Copy link
Contributor

merks commented Dec 13, 2024

See, rinse, lather, repeat, and then all the builds are good:

image

@merks merks merged commit b7d5b14 into eclipse-platform:master Dec 13, 2024
17 checks passed
* @return the auto edit strategies or <code>null</code> if automatic editing is not to be enabled
* @since 3.1
*/
public IAutoEditStrategy[] getAutoEditStrategies(ISourceViewer sourceViewer, String contentType) {
Copy link
Member

@sratz sratz Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the javadoc of this method says

For backward compatibility, this implementation always returns an array containing the result of {@link #getAutoIndentStrategy(ISourceViewer, String)}.

So we are now breaking this compatibility contract?

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

Labels

bug Something isn't working regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show source on hover & Declaration view broken

7 participants