-
Notifications
You must be signed in to change notification settings - Fork 775
Fix clipboard copy formatting based on selection dimensions in Stack Viewer #2332
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
Changes from 4 commits
c40f773
2cc6f15
a07effc
afbc9d4
079946b
2bb092a
2b68cf2
11214cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,16 +63,23 @@ public PerfDataGrid() | |
| morphedContent = PadForColumn(morphedContent, i + e.StartColumnDisplayIndex); | ||
| } | ||
|
|
||
| // Add a leading | character to the first column to ensure GitHub renders the content as table | ||
| if (i == 0) | ||
| // Add markdown table formatting (| symbols) only when: | ||
| // - Multiple columns AND multiple rows selected | ||
| // Single column (any rows) or multiple columns in single row: no pipes | ||
| bool shouldAddPipes = (m_numSelectedColumns > 1 && m_numSelectedRows > 1); | ||
| if (shouldAddPipes) | ||
| { | ||
| morphedContent = "| " + morphedContent; | ||
| } | ||
|
|
||
| // Add a trailing | character to the last column to complete the markdown table row | ||
| if (i == e.ClipboardRowContent.Count - 1) | ||
| { | ||
| morphedContent = morphedContent + " |"; | ||
| // Add a leading | character to the first column to ensure GitHub renders the content as table | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove reference to GitHub - this is just a markdown table.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in commit 2b68cf2. Removed the GitHub reference and updated the comment to simply say "for markdown table format". |
||
| if (i == 0) | ||
|
marklio marked this conversation as resolved.
Outdated
|
||
| { | ||
| morphedContent = "| " + morphedContent; | ||
| } | ||
|
|
||
| // Add a trailing | character to the last column to complete the markdown table row | ||
| if (i == e.ClipboardRowContent.Count - 1) | ||
| { | ||
| morphedContent = morphedContent + " |"; | ||
| } | ||
| } | ||
|
|
||
| // TODO Ugly, morph two cells on different rows into one line for the correct cut/paste experience | ||
|
|
@@ -520,27 +527,71 @@ private void SelectedCellsChanged(object sender, SelectedCellsChangedEventArgs e | |
| { | ||
| // We don't want the header for single values, or for 2 (for cutting and pasting ranges). | ||
| int numSelectedCells = window.SelectedCellsChanged(sender, e); | ||
| if (numSelectedCells <= 2) | ||
| m_numSelectedCells = numSelectedCells; | ||
|
|
||
| // Calculate the number of unique columns and rows selected | ||
| var dataGrid = sender as DataGrid; | ||
| if (dataGrid != null && dataGrid.SelectedCells.Count > 0) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: if (sender is Datagrid dataGrid && dataGrid.SelectedCells.Count > 0) While this is a personal preference, it does prevent duplicative checks and also scopes dataGrid to only scenarios in which it isn't null.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this, but didn't implement it for this particular one because we depend on the variable later in the method and I didn't want to duplicate the cast. |
||
| { | ||
| if (numSelectedCells == 2) | ||
| var uniqueColumns = new HashSet<DataGridColumn>(); | ||
| var uniqueRows = new HashSet<object>(); | ||
| foreach (var cell in dataGrid.SelectedCells) | ||
| { | ||
| var dataGrid = sender as DataGrid; | ||
| if (dataGrid != null) | ||
| { | ||
| var cells = dataGrid.SelectedCells; | ||
| if (cells != null) | ||
| { | ||
| m_clipboardRangeStart = GetCellStringValue(cells[0]); | ||
| m_clipboardRangeEnd = GetCellStringValue(cells[1]); | ||
| } | ||
| } | ||
| uniqueColumns.Add(cell.Column); | ||
| uniqueRows.Add(cell.Item); | ||
| } | ||
| Grid.ClipboardCopyMode = DataGridClipboardCopyMode.ExcludeHeader; | ||
| m_numSelectedColumns = uniqueColumns.Count; | ||
| m_numSelectedRows = uniqueRows.Count; | ||
| } | ||
| else | ||
| { | ||
| m_numSelectedColumns = 0; | ||
| m_numSelectedRows = 0; | ||
| } | ||
|
|
||
| // Determine whether to include headers based on selection: | ||
| // - Single cell: no header | ||
| // - 2 cells (range): no header | ||
| // - Single column, multiple cells: include header | ||
| // - Multiple columns, single row: no header | ||
| // - Multiple columns, multiple rows: include header | ||
| bool shouldIncludeHeader = false; | ||
| if (numSelectedCells > 2) | ||
| { | ||
| if (m_numSelectedColumns == 1) | ||
| { | ||
| // Single column, multiple rows: include header | ||
| shouldIncludeHeader = true; | ||
| } | ||
| else if (m_numSelectedRows > 1) | ||
| { | ||
| // Multiple columns, multiple rows: include header | ||
| shouldIncludeHeader = true; | ||
| } | ||
| // Multiple columns, single row: no header (shouldIncludeHeader stays false) | ||
| } | ||
|
|
||
| if (shouldIncludeHeader) | ||
| { | ||
| Grid.ClipboardCopyMode = DataGridClipboardCopyMode.IncludeHeader; | ||
| } | ||
| else | ||
| { | ||
| Grid.ClipboardCopyMode = DataGridClipboardCopyMode.ExcludeHeader; | ||
| } | ||
|
|
||
| if (numSelectedCells == 2) | ||
| { | ||
| if (dataGrid != null) | ||
| { | ||
| var cells = dataGrid.SelectedCells; | ||
| if (cells != null) | ||
| { | ||
| m_clipboardRangeStart = GetCellStringValue(cells[0]); | ||
| m_clipboardRangeEnd = GetCellStringValue(cells[1]); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| m_maxColumnInSelection = null; | ||
| } | ||
|
|
@@ -559,6 +610,9 @@ private void DoHyperlinkHelp(object sender, System.Windows.RoutedEventArgs e) | |
| /// </summary> | ||
| private string m_clipboardRangeStart; | ||
| private string m_clipboardRangeEnd; | ||
| private int m_numSelectedCells; | ||
| private int m_numSelectedColumns; | ||
| private int m_numSelectedRows; | ||
| private int[] m_maxColumnInSelection; | ||
| private int m_FindEnd; | ||
| private Regex m_findPat; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do want to add pipe symbols for single rows when there are multiple columns selected. The one case where we don't want to add pipe symbols is when there is one row selected, two columns selected, and those cells are the last two columns (First and Last). We should detect this and adjust the value of
shouldAddPipes.Also, down below, we should update Grid.ClipboardCopyMode for all cases except the following:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 079946b. Now: