Add user preference for auto-selecting a base for merges
For reviewing merge commits most users expect to the see the full diff that is integrated by accepting the merge commit. This diff is available if the user selects 'Parent 1' from the 'Diff against' drop-down list, however this is a new feature and most users don't know about. Also it is inconvenient to manually select the first parent for every merge. To make this easier add a user preferences for automatically selecting a base for merges. Let the default for this new preference be 'First Parent', as this is what most users expect. Change-Id: I633291f78fee7f9ce1feb73deb81f9312363b347 Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
parent
82cdfdf403
commit
d540a257ed
|
@ -644,6 +644,23 @@ on comments that you write yourself.
|
||||||
+
|
+
|
||||||
Email notifications are disabled.
|
Email notifications are disabled.
|
||||||
|
|
||||||
|
- [[default-base-for-merges]]`Default Base For Merges`:
|
||||||
|
+
|
||||||
|
This setting controls which base should be pre-selected in the
|
||||||
|
`Diff Against` drop-down list when the change screen is opened for a
|
||||||
|
merge commit.
|
||||||
|
+
|
||||||
|
** `Auto Merge`:
|
||||||
|
+
|
||||||
|
Pre-selects `Auto Merge` in the `Diff Against` drop-down list when the
|
||||||
|
change screen is opened for a merge commit.
|
||||||
|
+
|
||||||
|
** `First Parent`:
|
||||||
|
+
|
||||||
|
Pre-selects `Parent 1` in the `Diff Against` drop-down list when the
|
||||||
|
change screen is opened for a merge commit.
|
||||||
|
+
|
||||||
|
|
||||||
- [[diff-view]]`Diff View`:
|
- [[diff-view]]`Diff View`:
|
||||||
+
|
+
|
||||||
Whether the Side-by-Side diff view or the Unified diff view should be
|
Whether the Side-by-Side diff view or the Unified diff view should be
|
||||||
|
|
|
@ -1213,6 +1213,7 @@ any account.
|
||||||
"size_bar_in_change_table": true,
|
"size_bar_in_change_table": true,
|
||||||
"review_category_strategy": "ABBREV",
|
"review_category_strategy": "ABBREV",
|
||||||
"mute_common_path_prefixes": true,
|
"mute_common_path_prefixes": true,
|
||||||
|
"default_base_for_merges": "FIRST_PARENT",
|
||||||
"my": [
|
"my": [
|
||||||
{
|
{
|
||||||
"url": "#/dashboard/self",
|
"url": "#/dashboard/self",
|
||||||
|
@ -2469,6 +2470,10 @@ from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
|
||||||
their own comments. On `DISABLED` the user will not receive any email
|
their own comments. On `DISABLED` the user will not receive any email
|
||||||
notifications from Gerrit.
|
notifications from Gerrit.
|
||||||
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
|
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
|
||||||
|
|`default_base_for_merges` ||
|
||||||
|
The base which should be pre-selected in the 'Diff Against' drop-down
|
||||||
|
list when the change screen is opened for a merge commit.
|
||||||
|
Allowed values are `AUTO_MERGE` and `FIRST_PARENT`.
|
||||||
|============================================
|
|============================================
|
||||||
|
|
||||||
[[preferences-input]]
|
[[preferences-input]]
|
||||||
|
@ -2527,6 +2532,10 @@ from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
|
||||||
their own comments. On `DISABLED` the user will not receive any email
|
their own comments. On `DISABLED` the user will not receive any email
|
||||||
notifications from Gerrit.
|
notifications from Gerrit.
|
||||||
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
|
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
|
||||||
|
|`default_base_for_merges` |optional|
|
||||||
|
The base which should be pre-selected in the 'Diff Against' drop-down
|
||||||
|
list when the change screen is opened for a merge commit.
|
||||||
|
Allowed values are `AUTO_MERGE` and `FIRST_PARENT`.
|
||||||
|============================================
|
|============================================
|
||||||
|
|
||||||
[[query-limit-info]]
|
[[query-limit-info]]
|
||||||
|
|
|
@ -22,6 +22,7 @@ import com.google.gerrit.acceptance.NoHttpd;
|
||||||
import com.google.gerrit.acceptance.TestAccount;
|
import com.google.gerrit.acceptance.TestAccount;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
|
||||||
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
|
||||||
|
@ -87,6 +88,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
|
||||||
i.dateFormat = DateFormat.US;
|
i.dateFormat = DateFormat.US;
|
||||||
i.timeFormat = TimeFormat.HHMM_24;
|
i.timeFormat = TimeFormat.HHMM_24;
|
||||||
i.emailStrategy = EmailStrategy.DISABLED;
|
i.emailStrategy = EmailStrategy.DISABLED;
|
||||||
|
i.defaultBaseForMerges = DefaultBase.AUTO_MERGE;
|
||||||
i.relativeDateInChangeTable ^= true;
|
i.relativeDateInChangeTable ^= true;
|
||||||
i.sizeBarInChangeTable ^= true;
|
i.sizeBarInChangeTable ^= true;
|
||||||
i.legacycidInChangeTable ^= true;
|
i.legacycidInChangeTable ^= true;
|
||||||
|
|
|
@ -83,6 +83,25 @@ public class GeneralPreferencesInfo {
|
||||||
DISABLED
|
DISABLED
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public enum DefaultBase {
|
||||||
|
AUTO_MERGE(null),
|
||||||
|
FIRST_PARENT(-1);
|
||||||
|
|
||||||
|
private final String base;
|
||||||
|
|
||||||
|
DefaultBase(String base) {
|
||||||
|
this.base = base;
|
||||||
|
}
|
||||||
|
|
||||||
|
DefaultBase(int base) {
|
||||||
|
this(Integer.toString(base));
|
||||||
|
}
|
||||||
|
|
||||||
|
public String getBase() {
|
||||||
|
return base;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public enum TimeFormat {
|
public enum TimeFormat {
|
||||||
/** 12-hour clock: 1:15 am, 2:13 pm */
|
/** 12-hour clock: 1:15 am, 2:13 pm */
|
||||||
HHMM_12("h:mm a"),
|
HHMM_12("h:mm a"),
|
||||||
|
@ -123,6 +142,7 @@ public class GeneralPreferencesInfo {
|
||||||
public List<MenuItem> my;
|
public List<MenuItem> my;
|
||||||
public Map<String, String> urlAliases;
|
public Map<String, String> urlAliases;
|
||||||
public EmailStrategy emailStrategy;
|
public EmailStrategy emailStrategy;
|
||||||
|
public DefaultBase defaultBaseForMerges;
|
||||||
|
|
||||||
public boolean isShowInfoInReviewCategory() {
|
public boolean isShowInfoInReviewCategory() {
|
||||||
return getReviewCategoryStrategy() != ReviewCategoryStrategy.NONE;
|
return getReviewCategoryStrategy() != ReviewCategoryStrategy.NONE;
|
||||||
|
@ -180,6 +200,7 @@ public class GeneralPreferencesInfo {
|
||||||
p.legacycidInChangeTable = false;
|
p.legacycidInChangeTable = false;
|
||||||
p.muteCommonPathPrefixes = true;
|
p.muteCommonPathPrefixes = true;
|
||||||
p.signedOffBy = false;
|
p.signedOffBy = false;
|
||||||
|
p.defaultBaseForMerges = DefaultBase.FIRST_PARENT;
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,6 +19,7 @@ import com.google.gerrit.client.rpc.NativeString;
|
||||||
import com.google.gerrit.client.rpc.Natives;
|
import com.google.gerrit.client.rpc.Natives;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
|
||||||
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
|
||||||
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
|
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
|
||||||
|
@ -55,6 +56,7 @@ public class GeneralPreferences extends JavaScriptObject {
|
||||||
p.reviewCategoryStrategy(d.getReviewCategoryStrategy());
|
p.reviewCategoryStrategy(d.getReviewCategoryStrategy());
|
||||||
p.diffView(d.getDiffView());
|
p.diffView(d.getDiffView());
|
||||||
p.emailStrategy(d.emailStrategy);
|
p.emailStrategy(d.emailStrategy);
|
||||||
|
p.defaultBaseForMerges(d.defaultBaseForMerges);
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -135,6 +137,14 @@ public class GeneralPreferences extends JavaScriptObject {
|
||||||
private native String emailStrategyRaw()
|
private native String emailStrategyRaw()
|
||||||
/*-{ return this.email_strategy }-*/;
|
/*-{ return this.email_strategy }-*/;
|
||||||
|
|
||||||
|
public final DefaultBase defaultBaseForMerges() {
|
||||||
|
String s = defaultBaseForMergesRaw();
|
||||||
|
return s != null ? DefaultBase.valueOf(s) : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
private native String defaultBaseForMergesRaw()
|
||||||
|
/*-{ return this.default_base_for_merges }-*/;
|
||||||
|
|
||||||
public final native JsArray<TopMenuItem> my()
|
public final native JsArray<TopMenuItem> my()
|
||||||
/*-{ return this.my; }-*/;
|
/*-{ return this.my; }-*/;
|
||||||
|
|
||||||
|
@ -201,6 +211,12 @@ public class GeneralPreferences extends JavaScriptObject {
|
||||||
private native void emailStrategyRaw(String s)
|
private native void emailStrategyRaw(String s)
|
||||||
/*-{ this.email_strategy = s }-*/;
|
/*-{ this.email_strategy = s }-*/;
|
||||||
|
|
||||||
|
public final void defaultBaseForMerges(DefaultBase b) {
|
||||||
|
defaultBaseForMergesRaw(b != null ? b.toString() : null);
|
||||||
|
}
|
||||||
|
private native void defaultBaseForMergesRaw(String b)
|
||||||
|
/*-{ this.default_base_for_merges = b }-*/;
|
||||||
|
|
||||||
public final void setMyMenus(List<TopMenuItem> myMenus) {
|
public final void setMyMenus(List<TopMenuItem> myMenus) {
|
||||||
initMy();
|
initMy();
|
||||||
for (TopMenuItem n : myMenus) {
|
for (TopMenuItem n : myMenus) {
|
||||||
|
|
|
@ -168,4 +168,8 @@ public interface AccountConstants extends Constants {
|
||||||
String messageCCMeOnMyComments();
|
String messageCCMeOnMyComments();
|
||||||
String messageDisabled();
|
String messageDisabled();
|
||||||
String emailFieldLabel();
|
String emailFieldLabel();
|
||||||
|
|
||||||
|
String defaultBaseForMerges();
|
||||||
|
String autoMerge();
|
||||||
|
String firstParent();
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,6 +19,10 @@ messageEnabled = Enabled
|
||||||
messageCCMeOnMyComments = CC Me On Comments I Write
|
messageCCMeOnMyComments = CC Me On Comments I Write
|
||||||
messageDisabled = Disabled
|
messageDisabled = Disabled
|
||||||
|
|
||||||
|
defaultBaseForMerges = Default Base For Merges:
|
||||||
|
autoMerge = Auto Merge
|
||||||
|
firstParent = First Parent
|
||||||
|
|
||||||
maximumPageSizeFieldLabel = Maximum Page Size:
|
maximumPageSizeFieldLabel = Maximum Page Size:
|
||||||
diffViewLabel = Diff View:
|
diffViewLabel = Diff View:
|
||||||
dateFormatLabel = Date/Time Format:
|
dateFormatLabel = Date/Time Format:
|
||||||
|
|
|
@ -61,6 +61,7 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
private ListBox reviewCategoryStrategy;
|
private ListBox reviewCategoryStrategy;
|
||||||
private ListBox diffView;
|
private ListBox diffView;
|
||||||
private ListBox emailStrategy;
|
private ListBox emailStrategy;
|
||||||
|
private ListBox defaultBaseForMerges;
|
||||||
private StringListPanel myMenus;
|
private StringListPanel myMenus;
|
||||||
private Button save;
|
private Button save;
|
||||||
|
|
||||||
|
@ -106,6 +107,12 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
GeneralPreferencesInfo.EmailStrategy.DISABLED
|
GeneralPreferencesInfo.EmailStrategy.DISABLED
|
||||||
.name());
|
.name());
|
||||||
|
|
||||||
|
defaultBaseForMerges = new ListBox();
|
||||||
|
defaultBaseForMerges.addItem(Util.C.autoMerge(),
|
||||||
|
GeneralPreferencesInfo.DefaultBase.AUTO_MERGE.name());
|
||||||
|
defaultBaseForMerges.addItem(Util.C.firstParent(),
|
||||||
|
GeneralPreferencesInfo.DefaultBase.FIRST_PARENT.name());
|
||||||
|
|
||||||
diffView = new ListBox();
|
diffView = new ListBox();
|
||||||
diffView.addItem(
|
diffView.addItem(
|
||||||
com.google.gerrit.client.changes.Util.C.sideBySide(),
|
com.google.gerrit.client.changes.Util.C.sideBySide(),
|
||||||
|
@ -156,7 +163,7 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
signedOffBy = new CheckBox(Util.C.signedOffBy());
|
signedOffBy = new CheckBox(Util.C.signedOffBy());
|
||||||
|
|
||||||
boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled();
|
boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled();
|
||||||
final Grid formGrid = new Grid(12 + (flashClippy ? 1 : 0), 2);
|
final Grid formGrid = new Grid(13 + (flashClippy ? 1 : 0), 2);
|
||||||
|
|
||||||
int row = 0;
|
int row = 0;
|
||||||
|
|
||||||
|
@ -176,6 +183,10 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
formGrid.setWidget(row, fieldIdx, emailStrategy);
|
formGrid.setWidget(row, fieldIdx, emailStrategy);
|
||||||
row++;
|
row++;
|
||||||
|
|
||||||
|
formGrid.setText(row, labelIdx, Util.C.defaultBaseForMerges());
|
||||||
|
formGrid.setWidget(row, fieldIdx, defaultBaseForMerges);
|
||||||
|
row++;
|
||||||
|
|
||||||
formGrid.setText(row, labelIdx, Util.C.diffViewLabel());
|
formGrid.setText(row, labelIdx, Util.C.diffViewLabel());
|
||||||
formGrid.setWidget(row, fieldIdx, diffView);
|
formGrid.setWidget(row, fieldIdx, diffView);
|
||||||
row++;
|
row++;
|
||||||
|
@ -239,6 +250,7 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
e.listenTo(diffView);
|
e.listenTo(diffView);
|
||||||
e.listenTo(reviewCategoryStrategy);
|
e.listenTo(reviewCategoryStrategy);
|
||||||
e.listenTo(emailStrategy);
|
e.listenTo(emailStrategy);
|
||||||
|
e.listenTo(defaultBaseForMerges);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
@ -272,6 +284,7 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
reviewCategoryStrategy.setEnabled(on);
|
reviewCategoryStrategy.setEnabled(on);
|
||||||
diffView.setEnabled(on);
|
diffView.setEnabled(on);
|
||||||
emailStrategy.setEnabled(on);
|
emailStrategy.setEnabled(on);
|
||||||
|
defaultBaseForMerges.setEnabled(on);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void display(GeneralPreferences p) {
|
private void display(GeneralPreferences p) {
|
||||||
|
@ -296,6 +309,9 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
setListBox(emailStrategy,
|
setListBox(emailStrategy,
|
||||||
GeneralPreferencesInfo.EmailStrategy.ENABLED,
|
GeneralPreferencesInfo.EmailStrategy.ENABLED,
|
||||||
p.emailStrategy());
|
p.emailStrategy());
|
||||||
|
setListBox(defaultBaseForMerges,
|
||||||
|
GeneralPreferencesInfo.DefaultBase.FIRST_PARENT,
|
||||||
|
p.defaultBaseForMerges());
|
||||||
display(p.my());
|
display(p.my());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -385,6 +401,10 @@ public class MyPreferencesScreen extends SettingsScreen {
|
||||||
GeneralPreferencesInfo.EmailStrategy.ENABLED,
|
GeneralPreferencesInfo.EmailStrategy.ENABLED,
|
||||||
GeneralPreferencesInfo.EmailStrategy.values()));
|
GeneralPreferencesInfo.EmailStrategy.values()));
|
||||||
|
|
||||||
|
p.defaultBaseForMerges(getListBox(defaultBaseForMerges,
|
||||||
|
GeneralPreferencesInfo.DefaultBase.FIRST_PARENT,
|
||||||
|
GeneralPreferencesInfo.DefaultBase.values()));
|
||||||
|
|
||||||
List<TopMenuItem> items = new ArrayList<>();
|
List<TopMenuItem> items = new ArrayList<>();
|
||||||
for (List<String> v : myMenus.getValues()) {
|
for (List<String> v : myMenus.getValues()) {
|
||||||
items.add(TopMenuItem.create(v.get(0), v.get(1)));
|
items.add(TopMenuItem.create(v.get(0), v.get(1)));
|
||||||
|
|
|
@ -287,13 +287,17 @@ public class ChangeScreen extends Screen {
|
||||||
info.init();
|
info.init();
|
||||||
addExtensionPoints(info, initCurrentRevision(info));
|
addExtensionPoints(info, initCurrentRevision(info));
|
||||||
|
|
||||||
RevisionInfo rev = info.revision(revision);
|
final RevisionInfo rev = info.revision(revision);
|
||||||
CallbackGroup group = new CallbackGroup();
|
CallbackGroup group = new CallbackGroup();
|
||||||
loadCommit(rev, group);
|
loadCommit(rev, group);
|
||||||
|
|
||||||
group.addListener(new GerritCallback<Void>() {
|
group.addListener(new GerritCallback<Void>() {
|
||||||
@Override
|
@Override
|
||||||
public void onSuccess(Void result) {
|
public void onSuccess(Void result) {
|
||||||
|
if (base == null && rev.commit().parents().length() > 1) {
|
||||||
|
base = Gerrit.getUserPreferences()
|
||||||
|
.defaultBaseForMerges().getBase();
|
||||||
|
}
|
||||||
loadConfigInfo(info, base);
|
loadConfigInfo(info, base);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
|
@ -136,6 +136,8 @@ public class Schema_119 extends SchemaVersion {
|
||||||
p.reviewCategoryStrategy =
|
p.reviewCategoryStrategy =
|
||||||
toReviewCategoryStrategy(rs.getString(14));
|
toReviewCategoryStrategy(rs.getString(14));
|
||||||
p.muteCommonPathPrefixes = toBoolean(rs.getString(15));
|
p.muteCommonPathPrefixes = toBoolean(rs.getString(15));
|
||||||
|
p.defaultBaseForMerges =
|
||||||
|
GeneralPreferencesInfo.defaults().defaultBaseForMerges;
|
||||||
imports.put(accountId, p);
|
imports.put(accountId, p);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue