feat: add `PID` column to `Process` table (#379)

* feat: add `PID` column to `Process` table

* fix(tests): populate fake data with the correct `ProcessInfo` type

* test: update snapshots

* refactor: use more idiomatic rust

* refactor: rename function from `get_proc_name` to `get_proc_info`

* refactor: only display PID when width available is highest

ref: https://github.com/imsnif/bandwhich/pull/165#issuecomment-620852892

* tests: update snapshots

* chore: update CHANGELOG

* fix: clippy warnings

* Revert "fix: clippy warnings"

This reverts commit e5f06cba1943d60b43a19957c42e3178c6916f69.
We will do this separately for the sake of keeping a clean history

* refactor: use `u32` for PID

* refactor: more idiomatic rust

---------

Co-authored-by: cyqsimon <28627918+cyqsimon@users.noreply.github.com>

39 files changed, 219 insertions, 790 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0c8b4e3..38e910f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## Added
 * CI: include generated assets in release archive #359 - @cyqsimon
+* Add PID column to the process table #379 - @notjedi
 
 ## Changed
 * CI: strip release binaries for all targets #358 - @cyqsimon
diff --git a/src/display/components/table.rs b/src/display/components/table.rs
index 59193a0..0f1e7fd 100644
--- a/src/display/components/table.rs
+++ b/src/display/components/table.rs
@@ -28,7 +28,10 @@ pub enum DisplayLayout {
     C2([u16; 2]),
     /// Show 3 columns.
     C3([u16; 3]),
+    /// Show 4 columns.
+    C4([u16; 4]),
 }
+
 impl Index<usize> for DisplayLayout {
     type Output = u16;
 
@@ -36,28 +39,35 @@ impl Index<usize> for DisplayLayout {
         match self {
             Self::C2(arr) => &arr[i],
             Self::C3(arr) => &arr[i],
+            Self::C4(arr) => &arr[i],
         }
     }
 }
+
 impl DisplayLayout {
     #[inline]
     fn columns_count(&self) -> usize {
         match self {
             Self::C2(_) => 2,
             Self::C3(_) => 3,
+            Self::C4(_) => 4,
         }
     }
+
     #[inline]
     fn iter(&self) -> impl Iterator<Item = &u16> {
         match self {
             Self::C2(ws) => ws.iter(),
             Self::C3(ws) => ws.iter(),
+            Self::C4(ws) => ws.iter(),
         }
     }
+
     #[inline]
     fn widths_sum(&self) -> u16 {
         self.iter().sum()
     }
+
     /// Returns the computed actual width and the spacer width.
     ///
     /// See [`Table`] for layout rules.
@@ -87,6 +97,17 @@ impl DisplayLayout {
                 let w2_new = (w2 as f64 * m).trunc() as u16;
                 Self::C3([available_without_spacers - w1_new - w2_new, w1_new, w2_new])
             }
+            Self::C4([_w0, w1, w2, w3]) => {
+                let w1_new = (w1 as f64 * m).trunc() as u16;
+                let w2_new = (w2 as f64 * m).trunc() as u16;
+                let w3_new = (w3 as f64 * m).trunc() as u16;
+                Self::C4([
+                    available_without_spacers - w1_new - w2_new - w3_new,
+                    w1_new,
+                    w2_new,
+                    w3_new,
+                ])
+            }
         };
 
         (computed, spacer)
@@ -101,26 +122,41 @@ impl DisplayLayout {
 enum TableData {
     /// A table with 3 columns.
     C3(NColsTableData<3>),
+    /// A table with 4 columns.
+    C4(NColsTableData<4>),
 }
+
 impl From<NColsTableData<3>> for TableData {
     fn from(data: NColsTableData<3>) -> Self {
         Self::C3(data)
     }
 }
+
+impl From<NColsTableData<4>> for TableData {
+    fn from(data: NColsTableData<4>) -> Self {
+        Self::C4(data)
+    }
+}
+
 impl TableData {
     fn column_names(&self) -> &[&str] {
         match self {
             Self::C3(inner) => &inner.column_names,
+            Self::C4(inner) => &inner.column_names,
         }
     }
+
     fn rows(&self) -> Vec<&[String]> {
         match self {
             Self::C3(inner) => inner.rows.iter().map(|r| r.as_slice()).collect(),
+            Self::C4(inner) => inner.rows.iter().map(|r| r.as_slice()).collect(),
         }
     }
+
     fn column_selector(&self) -> &dyn Fn(&DisplayLayout) -> Vec<usize> {
         match self {
             Self::C3(inner) => inner.column_selector.as_ref(),
+            Self::C4(inner) => inner.column_selector.as_ref(),
         }
     }
 }
@@ -171,6 +207,7 @@ pub struct Table {
     width_cutoffs: Vec<(u16, DisplayLayout)>,
     data: TableData,
 }
+
 impl Table {
     pub fn create_connections_table(state: &UIState, ip_to_host: &HashMap<IpAddr, String>) -> Self {
         use DisplayLayout as D;
@@ -214,6 +251,7 @@ impl Table {
         let column_selector = Rc::new(|layout: &D| match layout {
             D::C2(_) => vec![0, 2],
             D::C3(_) => vec![0, 1, 2],
+            D::C4(_) => unreachable!(),
         });
 
         Table {
@@ -236,11 +274,12 @@ impl Table {
             (0, D::C2([16, 18])),
             (50, D::C3([16, 12, 20])),
             (60, D::C3([24, 12, 20])),
-            (80, D::C3([36, 16, 24])),
+            (80, D::C4([28, 12, 12, 24])),
         ];
 
         let column_names = [
             "Process",
+            "PID",
             "Connections",
             if state.cumulative_mode {
                 "Data (Up / Down)"
@@ -251,9 +290,10 @@ impl Table {
         let rows = state
             .processes
             .iter()
-            .map(|(process_name, data_for_process)| {
+            .map(|(proc_info, data_for_process)| {
                 [
-                    (*process_name).to_string(),
+                    proc_info.name.to_string(),
+                    proc_info.pid.to_string(),
                     data_for_process.connection_count.to_string(),
                     display_upload_and_download(
                         data_for_process,
@@ -264,8 +304,9 @@ impl Table {
             })
             .collect();
         let column_selector = Rc::new(|layout: &D| match layout {
-            D::C2(_) => vec![0, 2],
-            D::C3(_) => vec![0, 1, 2],
+            D::C2(_) => vec![0, 3],
+            D::C3(_) => vec![0, 2, 3],
+            D::C4(_) => vec![0, 1, 2, 3],
         });
 
         Table {
@@ -322,6 +363,7 @@ impl Table {
         let column_selector = Rc::new(|layout: &D| match layout {
             D::C2(_) => vec![0, 2],
             D::C3(_) => vec![0, 1, 2],
+            D::C4(_) => unreachable!(),
         });
 
         Table {
diff --git a/src/display/ui.rs b/src/display/ui.rs
index 4a2866e..e9df82d 100644
--- a/src/display/ui.rs
+++ b/src/display/ui.rs
@@ -10,6 +10,7 @@ use crate::{
         UIState,
     },
     network::{display_connection_string, display_ip_or_host, LocalSocket, Utilization},
+    os::ProcessInfo,
 };
 
 pub struct Ui<B>
@@ -53,9 +54,10 @@ where
 
         let output_process_data = |write_to_stdout: &mut (dyn FnMut(String) + Send),
                                    no_traffic: &mut bool| {
-            for (process, process_network_data) in &state.processes {
+            for (proc_info, process_network_data) in &state.processes {
                 write_to_stdout(format!(
-                    "process: <{timestamp}> \"{process}\" up/down Bps: {}/{} connections: {}",
+                    "process: <{timestamp}> \"{}\" up/down Bps: {}/{} connections: {}",
+                    proc_info.name,
                     process_network_data.total_bytes_uploaded,
                     process_network_data.total_bytes_downloaded,
                     process_network_data.connection_count
@@ -173,7 +175,7 @@ where
 
     pub fn update_state(
         &mut self,
-        connections_to_procs: HashMap<LocalSocket, String>,
+        connections_to_procs: HashMap<LocalSocket, ProcessInfo>,
         utilization: Utilization,
         ip_to_host: HashMap<IpAddr, String>,
     ) {
diff --git a/src/display/ui_state.rs b/src/display/ui_state.rs
index 8b73571..20085d0 100644
--- a/src/display/ui_state.rs
+++ b/src/display/ui_state.rs
@@ -9,6 +9,7 @@ use crate::{
     display::BandwidthUnitFamily,
     mt_log,
     network::{Connection, LocalSocket, Utilization},
+    os::ProcessInfo,
 };
 
 static RECALL_LENGTH: usize = 5;
@@ -72,7 +73,7 @@ impl Bandwidth for ConnectionData {
 }
 
 pub struct UtilizationData {
-    connections_to_procs: HashMap<LocalSocket, String>,
+    connections_to_procs: HashMap<LocalSocket, ProcessInfo>,
     network_utilization: Utilization,
 }
 
@@ -80,7 +81,7 @@ pub struct UtilizationData {
 pub struct UIState {
     /// The interface name in single-interface mode. `None` means all interfaces.
     pub interface_name: Option<String>,
-    pub processes: Vec<(String, NetworkData)>,
+    pub processes: Vec<(ProcessInfo, NetworkData)>,
     pub remote_addresses: Vec<(IpAddr, NetworkData)>,
     pub connections: Vec<(Connection, ConnectionData)>,
     pub total_bytes_downloaded: u128,
@@ -88,7 +89,7 @@ pub struct UIState {
     pub cumulative_mode: bool,
     pub unit_family: BandwidthUnitFamily,
     pub utilization_data: VecDeque<UtilizationData>,
-    pub processes_map: HashMap<String, NetworkData>,
+    pub processes_map: HashMap<ProcessInfo, NetworkData>,
     pub remote_addresses_map: HashMap<IpAddr, NetworkData>,
     pub connections_map: HashMap<Connection, ConnectionData>,
     /// Used for reducing logging noise.
@@ -98,7 +99,7 @@ pub struct UIState {
 impl UIState {
     pub fn update(
         &mut self,
-        connections_to_procs: HashMap<LocalSocket, String>,
+        connections_to_procs: HashMap<LocalSocket, ProcessInfo>,
         network_utilization: Utilization,
     ) {
         self.utilization_data.push_back(UtilizationData {
@@ -108,7 +109,7 @@ impl UIState {
         if self.utilization_data.len() > RECALL_LENGTH {
             self.utilization_data.pop_front();
         }
-        let mut processes: HashMap<String, NetworkData> = HashMap::new();
+        let mut processes: HashMap<ProcessInfo, NetworkData> = HashMap::new();
         let mut remote_addresses: HashMap<IpAddr, NetworkData> = HashMap::new();
         let mut connections: HashMap<Connection, ConnectionData> = HashMap::new();
         let mut total_bytes_downloaded: u128 = 0;
@@ -140,11 +141,10 @@ impl UIState {
 
                 let data_for_process = {
                     let local_socket = connection.local_socket;
-                    let process_name = get_proc_name(connections_to_procs, &local_socket);
+                    let proc_info = get_proc_info(connections_to_procs, &local_socket);
 
                     // only log each orphan connection once
-                    if process_name.is_none() && !self.known_orphan_sockets.contains(&local_socket)
-                    {
+                    if proc_info.is_none() && !self.known_orphan_sockets.contains(&local_socket) {
                         // newer connections go in the front so that searches are faster
                         // basically recency bias
                         self.known_orphan_sockets.push_front(local_socket);
@@ -155,17 +155,18 @@ impl UIState {
                             .find(|(&LocalSocket { port, protocol, .. }, _)| {
                                 port == local_socket.port && protocol == local_socket.protocol
                             })
-                            .and_then(|(local_conn_lookalike, name)| {
+                            .and_then(|(local_conn_lookalike, info)| {
                                 network_utilization
                                     .connections
                                     .keys()
                                     .find(|conn| &conn.local_socket == local_conn_lookalike)
-                                    .map(|conn| (conn, name))
+                                    .map(|conn| (conn, info))
                             }) {
-                            Some((lookalike, name)) => {
+                            Some((lookalike, proc_info)) => {
                                 mt_log!(
                                     warn,
-                                    r#""{name}" owns a similar looking connection, but its local ip doesn't match."#
+                                    r#""{0}" owns a similar looking connection, but its local ip doesn't match."#,
+                                    proc_info.name
                                 );
                                 mt_log!(warn, "Looking for: {connection:?}; found: {lookalike:?}");
                             }
@@ -175,9 +176,11 @@ impl UIState {
                         };
                     }
 
-                    let process_display_name = process_name.unwrap_or("<UNKNOWN>").to_owned();
-                    connection_data.process_name = process_display_name.clone();
-                    processes.entry(process_display_name).or_default()
+                    let proc_info = proc_info
+                        .cloned()
+                        .unwrap_or_else(|| ProcessInfo::new("<UNKNOWN>", 0));
+                    connection_data.process_name = proc_info.name.clone();
+                    processes.entry(proc_info).or_default()
                 };
 
                 data_for_process.total_bytes_downloaded += connection_info.total_bytes_downloaded;
@@ -221,10 +224,10 @@ impl UIState {
     }
 }
 
-fn get_proc_name<'a>(
-    connections_to_procs: &'a HashMap<LocalSocket, String>,
+fn get_proc_info<'a>(
+    connections_to_procs: &'a HashMap<LocalSocket, ProcessInfo>,
     local_socket: &LocalSocket,
-) -> Option<&'a str> {
+) -> Option<&'a ProcessInfo> {
     connections_to_procs
         // direct match
         .get(local_socket)
@@ -252,7 +255,6 @@ fn get_proc_name<'a>(
                 ..*local_socket
             })
         })
-        .map(String::as_str)
 }
 
 fn merge_bandwidth<K, V>(self_map: &mut HashMap<K, V>, other_map: HashMap<K, V>)
diff --git a/src/main.rs b/src/main.rs
index a6a00d9..756e46d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -34,6 +34,7 @@ use ratatui::backend::{Backend, CrosstermBackend};
 use simplelog::WriteLogger;
 
 use crate::cli::Opt;
+use crate::os::ProcessInfo;
 
 const DISPLAY_DELTA: Duration = Duration::from_millis(1000);
 
@@ -89,7 +90,7 @@ fn main() -> anyhow::Result<()> {
 }
 
 pub struct OpenSockets {
-    sockets_to_procs: HashMap<LocalSocket, String>,
+    sockets_to_procs: HashMap<LocalSocket, ProcessInfo>,
 }
 
 pub struct OsInputOutput {
diff --git a/src/os/linux.rs b/src/os/linux.rs
index d1813ab..fdd5ec1 100644
--- a/src/os/linux.rs
+++ b/src/os/linux.rs
@@ -4,6 +4,7 @@ use procfs::process::FDTarget;
 
 use crate::{
     network::{LocalSocket, Protocol},
+    os::ProcessInfo,
     OpenSockets,
 };
 
@@ -15,10 +16,11 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
         for process in all_procs.filter_map(|res| res.ok()) {
             let Ok(fds) = process.fd() else { continue };
             let Ok(stat) = process.stat() else { continue };
-            let procname = stat.comm;
+            let proc_name = stat.comm;
+            let proc_info = ProcessInfo::new(&proc_name, stat.pid as u32);
             for fd in fds.filter_map(|res| res.ok()) {
                 if let FDTarget::Socket(inode) = fd.target {
-                    inode_to_procname.insert(inode, procname.clone());
+                    inode_to_procname.insert(inode, proc_info.clone());
                 }
             }
         }
@@ -29,14 +31,14 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
             tcp.append(&mut tcp6);
         }
         for entry in tcp.into_iter() {
-            if let Some(procname) = inode_to_procname.get(&entry.inode) {
+            if let Some(proc_info) = inode_to_procname.get(&entry.inode) {
                 open_sockets.insert(
                     LocalSocket {
                         ip: entry.local_address.ip(),
                         port: entry.local_address.port(),
                         protocol: Protocol::Tcp,
                     },
-                    procname.clone(),
+                    proc_info.clone(),
                 );
             };
         }
@@ -47,14 +49,14 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
             udp.append(&mut udp6);
         }
         for entry in udp.into_iter() {
-            if let Some(procname) = inode_to_procname.get(&entry.inode) {
+            if let Some(proc_info) = inode_to_procname.get(&entry.inode) {
                 open_sockets.insert(
                     LocalSocket {
                         ip: entry.local_address.ip(),
                         port: entry.local_address.port(),
                         protocol: Protocol::Udp,
                     },
-                    procname.clone(),
+                    proc_info.clone(),
                 );
             };
         }
diff --git a/src/os/lsof.rs b/src/os/lsof.rs
index 088d9cf..aec444d 100644
--- a/src/os/lsof.rs
+++ b/src/os/lsof.rs
@@ -2,7 +2,7 @@ use crate::{os::lsof_utils::get_connections, OpenSockets};
 
 pub(crate) fn get_open_sockets() -> OpenSockets {
     let sockets_to_procs = get_connections()
-        .filter_map(|raw| raw.as_local_socket().map(|s| (s, raw.process_name)))
+        .filter_map(|raw| raw.as_local_socket().map(|s| (s, raw.proc_info)))
         .collect();
 
     OpenSockets { sockets_to_procs }
diff --git a/src/os/lsof_utils.rs b/src/os/lsof_utils.rs
index ad441e7..6fd61f8 100644
--- a/src/os/lsof_utils.rs
+++ b/src/os/lsof_utils.rs
@@ -6,6 +6,7 @@ use regex::Regex;
 use crate::{
     mt_log,
     network::{LocalSocket, Protocol},
+    os::ProcessInfo,
 };
 
 #[allow(dead_code)]
@@ -16,7 +17,7 @@ pub struct RawConnection {
     local_port: String,
     remote_port: String,
     protocol: String,
-    pub process_name: String,
+    pub proc_info: ProcessInfo,
 }
 
 fn get_null_addr(ip_type: &str) -> &str {
@@ -36,8 +37,9 @@ impl RawConnection {
             return None;
         }
         let process_name = columns[0].replace("\\x20", " ");
+        let pid = columns[1].parse().ok()?;
+        let proc_info = ProcessInfo::new(&process_name, pid);
         // Unneeded
-        // let pid = columns[1];
         // let username = columns[2];
         // let fd = columns[3];
 
@@ -74,7 +76,7 @@ impl RawConnection {
                 remote_ip,
                 remote_port,
                 protocol,
-                process_name,
+                proc_info,
             };
             Some(connection)
         } else if let Some(caps) = LISTEN_REGEX.captures(connection_str) {
@@ -97,7 +99,7 @@ impl RawConnection {
                 remote_ip,
                 remote_port,
                 protocol,
-                process_name,
+                proc_info,
             };
             Some(connection)
         } else {
@@ -118,7 +120,7 @@ impl RawConnection {
     }
 
     pub fn as_local_socket(&self) -> Option<LocalSocket> {
-        let process = &self.process_name;
+        let process = &self.proc_info.name;
 
         let Some(ip) = self.get_local_ip() else {
             mt_log!(
@@ -259,6 +261,6 @@ com.apple   590 etoledom  204u  IPv4 0x28ffb9c04111253f      0t0  TCP 192.168.1.
     }
     fn test_raw_connection_parse_process_name(raw_line: &str) {
         let connection = RawConnection::new(raw_line).unwrap();
-        assert_eq!(connection.process_name, String::from("ProcessName"));
+        assert_eq!(connection.proc_info.name, String::from("ProcessName"));
     }
 }
diff --git a/src/os/shared.rs b/src/os/shared.rs
index ed97878..d8dd477 100644
--- a/src/os/shared.rs
+++ b/src/os/shared.rs
@@ -19,6 +19,21 @@ use crate::os::lsof::get_open_sockets;
 #[cfg(target_os = "windows")]
 use crate::os::windows::get_open_sockets;
 
+#[derive(Clone, Debug, Default, Hash, PartialEq, Eq)]
+pub struct ProcessInfo {
+    pub name: String,
+    pub pid: u32,
+}
+
+impl ProcessInfo {
+    pub fn new(name: &str, pid: u32) -> Self {
+        Self {
+            name: name.to_string(),
+            pid,
+        }
+    }
+}
+
 pub struct TerminalEvents;
 
 impl Iterator for TerminalEvents {
diff --git a/src/os/windows.rs b/src/os/windows.rs
index a0d7823..620a4ba 100644
--- a/src/os/windows.rs
+++ b/src/os/windows.rs
@@ -5,6 +5,7 @@ use sysinfo::{Pid, System};
 
 use crate::{
     network::{LocalSocket, Protocol},
+    os::ProcessInfo,
     OpenSockets,
 };
 
@@ -20,13 +21,12 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
 
     if let Ok(sockets_info) = sockets_info {
         for si in sockets_info {
-            let mut procname = String::new();
-            for pid in si.associated_pids {
-                if let Some(process) = sysinfo.process(Pid::from_u32(pid)) {
-                    procname = String::from(process.name());
-                    break;
-                }
-            }
+            let proc_info = si
+                .associated_pids
+                .into_iter()
+                .find_map(|pid| sysinfo.process(Pid::from_u32(pid)))
+                .map(|p| ProcessInfo::new(p.name(), p.pid().as_u32()))
+                .unwrap_or_default();
 
             match si.protocol_socket_info {
                 ProtocolSocketInfo::Tcp(tcp_si) => {
@@ -36,7 +36,7 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
                             port: tcp_si.local_port,
                             protocol: Protocol::Tcp,
                         },
-                        procname,
+                        proc_info,
                     );
                 }
                 ProtocolSocketInfo::Udp(udp_si) => {
@@ -46,7 +46,7 @@ pub(crate) fn get_open_sockets() -> OpenSockets {
                             port: udp_si.local_port,
                             protocol: Protocol::Udp,
                         },
-                        procname,
+                        proc_info,
                     );
                 }
             }
diff --git a/src/tests/cases/snapshots/bandwhich__tests__cases__ui__basic_only_processes.snap b/src/tests/cases/snapshots/bandwhich__tests__cases__ui__basic_only_processes.snap
index 1910f6a..7eebb2f 100644
--- a/src/tests/cases/snapshots/bandwhich__tests__cases__ui__basic_only_processes.snap
+++ b/src/tests/cases/snapshots/bandwhich__tests__cases__ui__basic_only_processes.snap
@@ -4,7 +4,7 @@ expression: terminal_draw_events.lock().unwrap().join(SNAPSHOT_SECTION_SEPARATOR