Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2323 'Restore Defaults' uses different value when USE_MARKER_LIMITS pref is false. #2325

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

eobrienPilz
Copy link
Contributor

Proposed fix for issue #2323 is to use 100 as the default value on first launch and also on pressing the 'Restore Defaults' button.

Copy link
Contributor

github-actions bot commented Sep 25, 2024

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 31m 21s ⏱️ +13s
 7 734 tests ±0   7 506 ✅ ±0  228 💤 ±0  0 ❌ ±0 
24 363 runs  ±0  23 614 ✅ ±0  749 💤 ±0  0 ❌ ±0 

Results for commit 68b5f2b. ± Comparison against base commit d8afbe0.

♻️ This comment has been updated with latest results.

@eobrienPilz
Copy link
Contributor Author

Is anyone available to review this ?
Its just a small fix and shouldn't have any side effects.

@merks
Copy link
Contributor

merks commented Jan 2, 2025

I wonder if this really the correct fix. Isn't it possible for a product to override defaults?

Should it maybe be like this?

int markerLimits = preferenceStore.getInt(IDEInternalPreferences.MARKER_LIMITS_VALUE);

Indeed, this looks like an example of such customization:

image

@eobrienPilz
Copy link
Contributor Author

Yes, I am not sure if there is a reason why pressing restore defaults uses a hard coded value. I'll see if I can change it so that it always uses the stored default like you suggest.

@merks
Copy link
Contributor

merks commented Jan 2, 2025

Yes, the 1000 is definitely fishy. 😕

@eobrienPilz eobrienPilz force-pushed the 2323LimitDefaultValue branch from 9a7cab8 to 68b5f2b Compare January 2, 2025 17:15
@eobrienPilz
Copy link
Contributor Author

I made the update and tested it. The restore defaults button now sets the the stored preference value, as expected.

@merks merks merged commit 9ee216c into eclipse-platform:master Jan 2, 2025
17 checks passed
@merks
Copy link
Contributor

merks commented Jan 2, 2025

🙏

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.

2 participants