Merge e9cd8b1 into 0682e85

package/yast2-storage-ng.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Sun Nov 22 22:55:37 UTC 2020 - Martin Vidner <mvidner@suse.com>
+
+- Display pathnames correctly in a Right-to-Left context
+  (bsc#1128091).
+- 4.3.23
+
 -------------------------------------------------------------------
 Fri Nov 20 12:53:43 UTC 2020 - Josef Reidinger <jreidinger@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:           yast2-storage-ng
-Version:        4.3.22
+Version:        4.3.23
 Release:        0
 Summary:        YaST2 - Storage Configuration
 License:        GPL-2.0-only OR GPL-3.0-only

src/lib/y2partitioner/bidi.rb

@@ -0,0 +1,92 @@
+# Bidirectional Text: Left-to-right (Latin) and right-to-left (Arabic).
+#
+# See https://en.wikipedia.org/wiki/Bidirectional_text
+module Bidi
+  LRE = "\u{202A}".freeze
+  RLE = "\u{202B}".freeze
+  PDF = "\u{202C}".freeze
+  LRO = "\u{202D}".freeze
+  RLO = "\u{202E}".freeze
+  LRI = "\u{2066}".freeze
+  RLI = "\u{2067}".freeze
+  FSI = "\u{2068}".freeze
+  PDI = "\u{2069}".freeze
+
+  LEFT_TO_RIGHT_EMBEDDING = LRE
+  RIGHT_TO_LEFT_EMBEDDING = RLE
+  POP_DIRECTIONAL_FORMATTING = PDF
+  LEFT_TO_RIGHT_OVERRIDE = LRO
+  RIGHT_TO_LEFT_OVERRIDE = RLO
+  LEFT_TO_RIGHT_ISOLATE = LRI
+  RIGHT_TO_LEFT_ISOLATE = RLI
+  FIRST_STRONG_ISOLATE = FSI
+  POP_DIRECTIONAL_ISOLATE = PDI
+
+  module_function
+
+  # Wrap *str* in a pair of characters: Left-to-Right Embedding
+  def ltr_embed(str)
+    LRE + str + PDF
+  end
+
+  # Wrap *str* in a pair of characters: Right-to-Left Embedding
+  def rtl_embed(str)
+    RLE + str + PDF
+  end
+
+  # Wrap *str* in a pair of characters: Left-to-Right Override
+  def ltr_override(str)
+    LRO + str + PDF
+  end
+
+  # Wrap *str* in a pair of characters: Right-to-Left Override
+  def rtl_override(str)
+    RLO + str + PDF
+  end
+
+  # Wrap *str* in a pair of characters: Left-to-Right Isolate
+  def ltr_isolate(str)
+    LRI + str + PDI
+  end
+
+  # Wrap *str* in a pair of characters: Right-to-Left Isolate
+  def rtl_isolate(str)
+    RLI + str + PDI
+  end
+
+  # Wrap *str* in a pair of characters: First Strong Isolate
+  def first_strong_isolate(str)
+    FSI + str + PDI
+  end
+
+  BIDI_CONTROLS = LRE + RLE + PDF + LRO + RLO + LRI + RLI + FSI + PDI
+
+  LRM = "\u{200E}".freeze
+  RLM = "\u{200F}".freeze
+  ALM = "\u{061C}".freeze
+
+  LEFT_TO_RIGHT_MARK = LRM
+  RIGHT_TO_LEFT_MARK = RLM
+  ARABIC_LETTER_MARK = ALM
+
+  # Add bidi formatting characters to *pname*
+  # otherwise /dev/sda will be presented as dev/sda/ in RTL context
+  # @param pname [Pathname]
+  def pathname_bidi_to_s(pname)
+    return ltr_isolate(File::SEPARATOR) if pname.root?
+
+    isolated_components = pname.each_filename.map { |fn| first_strong_isolate(fn) }
+
+    isolated_components.unshift("") if pname.absolute?
+    joined = isolated_components.join(File::SEPARATOR) # "/" pedantry
+
+    ltr_isolate(joined)
+  end
+
+  # Return a copy of *str* where bidirectional formatting characters are removed
+  # @param str [String]
+  # @return [String]
+  def bidi_strip(str)
+    str.tr(BIDI_CONTROLS, "")
+  end
+end

src/lib/y2partitioner/widgets/columns/base.rb

@@ -20,6 +20,7 @@
 require "yast"
 require "abstract_method"
 require "y2partitioner/device_graphs"
+require "y2partitioner/bidi"
 
 module Y2Partitioner
   module Widgets
@@ -111,6 +112,23 @@ def id
 
         private
 
+        def left_to_right(path_string)
+          return path_string unless bidi_supported?
+
+          pn = Pathname.new(path_string)
+          Bidi.pathname_bidi_to_s(pn)
+        end
+
+        # In ncurses the bidi characters would mess up columns,
+        # making things worse; return false there.
+        def bidi_supported?
+          return @bidi_supported unless @bidi_supported.nil?
+
+          di = Yast::UI.GetDisplayInfo() || {}
+          text_mode = di.fetch("TextMode", true)
+          @bidi_supported = !text_mode
+        end
+
         # Helper method to create a `cell` term
         #
         # @param args [Array] content of the cell

src/lib/y2partitioner/widgets/columns/device.rb

@@ -53,10 +53,12 @@ def value_for(device, entry: nil)
 
         # The device name
         #
+        # @param entry [DeviceTableEntry]
         # @return [String]
         def device_name(device, entry)
-          return fstab_device_name(device, entry) if fstab_entry?(device)
-          return blk_device_name(device, entry) unless device.is?(:blk_filesystem)
+          return left_to_right(fstab_device_name(device, entry)) if fstab_entry?(device)
+          return left_to_right(blk_device_name(device, entry)) unless device.is?(:blk_filesystem)
+          # FS name
           return device.type.to_human_string unless device.multidevice?
 
           format(

src/lib/y2partitioner/widgets/columns/mount_point.rb

@@ -38,15 +38,18 @@ def title
 
         # @see Columns::Base#value_for
         def value_for(device)
-          return device.mount_point if fstab_entry?(device)
+          return left_to_right(device.mount_point) if fstab_entry?(device)
 
           filesystem = filesystem_for(device)
 
           return "" if filesystem.nil?
           return "" if part_of_multidevice?(device, filesystem)
 
-          res = filesystem.mount_path
-          res += " *" if filesystem.mount_point && !filesystem.mount_point.active?
+          fs_mp = filesystem.mount_point
+          return "" unless fs_mp
+
+          res = left_to_right(fs_mp.path)
+          res += " *" unless fs_mp.active?
           res
         end
       end

test/data/devicegraphs/bidi.yml

@@ -0,0 +1,29 @@
+---
+- disk:
+    name: /dev/sda
+    size: 1 TiB
+    file_system:  ext4
+    label:        root
+    mount_point:  "/"
+
+- disk:
+    name: /dev/sdb
+    size: 1 GiB
+    partition_table:  msdos
+    partitions:
+
+    - partition:
+        size:         500 MiB
+        name:         /dev/sdb1
+        file_system:  ext4
+        # /fidiu/qdima (/video/old)
+        mount_point:  "/\u0641\u064A\u062F\u064A\u0648/\u0642\u062F\u064A\u0645\u0629"
+
+    - partition:
+        size:         500 MiB
+        name:         /dev/sdb2
+        file_system:  ext4
+        # /fidiu/jdida (/video/new)
+        mount_point:  "/\u0641\u064A\u062F\u064A\u0648/\u062C\u062F\u064A\u062F\u0629"
+
+

test/y2partitioner/test_helper.rb

@@ -19,6 +19,7 @@
 # find current contact information at www.suse.com
 
 require_relative "../spec_helper"
+require "y2partitioner/bidi"
 
 # Removes the :sortKey term from a :cell term, possibly returning only
 # the single param of the term.
@@ -31,7 +32,7 @@ def remove_sort_key(term)
   term.params.delete_if { |param| param.is_a?(Yast::Term) && param.value == :sortKey }
   return term if term.params.size > 1
 
-  term.params[0]
+  Bidi.bidi_strip(term.params[0])
 end
 
 # Removes the :sortKey term from :cell terms somewhere inside (to the

test/y2partitioner/widgets/columns/device_test.rb

@@ -55,6 +55,15 @@
       it "uses the sort key provided by libstorage-ng" do
         expect(sort_key).to eq(device.name_sort_key)
       end
+
+      it "wraps the value with bidi control characters" do
+        allow(subject).to receive(:bidi_supported?).and_return(true)
+
+        expect(subject.value_for(device)).to satisfy do |term|
+          term.value == :cell &&
+            term.params[0] == "\u2066/\u2068dev\u2069/\u2068sdb1\u2069\u2069"
+        end
+      end
     end
 
     context "when the device is a single-device filesystem" do

test/y2partitioner/widgets/columns/mount_point_test.rb

@@ -36,13 +36,13 @@
     devicegraph_stub(scenario)
   end
 
-  describe "#values_for" do
+  describe "#value_for" do
     context "when given device is a fstab entry" do
       let(:btrfs) { Y2Storage::Filesystems::Type::BTRFS }
       let(:home_fstab_entry) { fstab_entry("/dev/sda2", "/home", btrfs, ["subvol=@/home"], 0, 0) }
 
       it "returns its mount point" do
-        expect(subject.value_for(home_fstab_entry)).to eq("/home")
+        expect(Bidi.bidi_strip(subject.value_for(home_fstab_entry))).to eq("/home")
       end
     end
 
@@ -69,7 +69,7 @@
       end
 
       it "returns the mount path without an asterisk sign" do
-        expect(subject.value_for(device)).to eq("/test2")
+        expect(Bidi.bidi_strip(subject.value_for(device))).to eq("/test2")
       end
     end
 
@@ -79,8 +79,35 @@
         Y2Storage::Filesystems::Nfs.find_by_server_and_path(devicegraph, "srv", "/home/a")
       end
 
-      it "returns the mount path including an asterkisk sign" do
-        expect(subject.value_for(device)).to eq("/test1 *")
+      it "returns the mount path including an asterisk sign" do
+        expect(Bidi.bidi_strip(subject.value_for(device))).to eq("/test1 *")
+      end
+    end
+
+    describe "bidi right to left (RTL) handling" do
+      let(:scenario) { "bidi.yml" }
+      before do
+        allow(subject).to receive(:bidi_supported?).and_return(true)
+      end
+      let(:device_name) { "/dev/sdb1" }
+
+      it "returns the mount path with appropriate bidi control characters" do
+        allow(subject).to receive(:bidi_supported?).and_return(true)
+
+        # \u20xx are the control characters
+        # \u06xx are Arabic letters
+        expect(subject.value_for(device)).to eq("\u2066" \
+                                                "/\u2068\u0641\u064A\u062F\u064A\u0648\u2069" \
+                                                "/\u2068\u0642\u062F\u064A\u0645\u0629\u2069" \
+                                                "\u2069")
+      end
+
+      context "when the path is root /" do
+        let(:device_name) { "/dev/sda" }
+
+        it "returns the mount path with wrapped with LTR Isolate" do
+          expect(subject.value_for(device)).to eq("\u2066/\u2069")
+        end
       end
     end
   end