widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #13479
[Merge] lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters into lp:widelands.
Commit message:
Added tooltip to file save and make directory edit boxes if illegal filename is being entered. Also, some refactoring:
- New function as_listitem in text layout
- Fixed tooltip drawing for modal panels
- Some code style tweaks to Panel
- Filename checks now live directly in Filesystem
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1610507 in widelands: "Add user info about disallowed characters in filenames"
https://bugs.launchpad.net/widelands/+bug/1610507
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1610507-disallowed-filename-characters/+merge/346442
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters into lp:widelands.
=== modified file 'src/editor/ui_menus/main_menu_save_map.cc'
--- src/editor/ui_menus/main_menu_save_map.cc 2018-04-27 06:11:05 +0000
+++ src/editor/ui_menus/main_menu_save_map.cc 2018-05-22 05:56:20 +0000
@@ -68,7 +68,8 @@
UI::ButtonStyle::kWuiPrimary,
_("Map Options")),
editbox_label_(
- this, padding_, tabley_ + tableh_ + 3 * padding_, butw_, buth_, _("Filename:")) {
+ this, padding_, tabley_ + tableh_ + 3 * padding_, butw_, buth_, _("Filename:")),
+ illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) {
set_current_directory(curdir_);
// Make room for edit_options_ button
@@ -212,7 +213,9 @@
*/
void MainMenuSaveMap::edit_box_changed() {
// Prevent the user from creating nonsense file names, like e.g. ".." or "...".
- ok_.set_enabled(LayeredFileSystem::is_legal_filename(editbox_->text()));
+ const bool is_legal_filename = FileSystem::is_legal_filename(editbox_->text());
+ ok_.set_enabled(is_legal_filename);
+ editbox_->set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_);
}
void MainMenuSaveMap::set_current_directory(const std::string& filename) {
=== modified file 'src/editor/ui_menus/main_menu_save_map.h'
--- src/editor/ui_menus/main_menu_save_map.h 2018-04-07 16:59:00 +0000
+++ src/editor/ui_menus/main_menu_save_map.h 2018-05-22 05:56:20 +0000
@@ -53,6 +53,7 @@
UI::Textarea editbox_label_;
UI::EditBox* editbox_;
+ const std::string illegal_filename_tooltip_;
};
#endif // end of include guard: WL_EDITOR_UI_MENUS_MAIN_MENU_SAVE_MAP_H
=== modified file 'src/editor/ui_menus/main_menu_save_map_make_directory.cc'
--- src/editor/ui_menus/main_menu_save_map_make_directory.cc 2018-04-27 06:11:05 +0000
+++ src/editor/ui_menus/main_menu_save_map_make_directory.cc 2018-05-22 05:56:20 +0000
@@ -54,7 +54,8 @@
butw_,
buth_,
UI::ButtonStyle::kWuiSecondary,
- _("Cancel")) {
+ _("Cancel")),
+ illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) {
vbox_.add(&label_);
vbox_.add_space(padding_);
@@ -80,6 +81,8 @@
void MainMenuSaveMapMakeDirectory::edit_changed() {
const std::string& text = edit_.text();
// Prevent the user from creating nonsense directory names, like e.g. ".." or "...".
- ok_button_.set_enabled(LayeredFileSystem::is_legal_filename(text));
+ const bool is_legal_filename = FileSystem::is_legal_filename(text);
+ ok_button_.set_enabled(is_legal_filename);
+ edit_.set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_);
dirname_ = text;
}
=== modified file 'src/editor/ui_menus/main_menu_save_map_make_directory.h'
--- src/editor/ui_menus/main_menu_save_map_make_directory.h 2018-04-07 16:59:00 +0000
+++ src/editor/ui_menus/main_menu_save_map_make_directory.h 2018-05-22 05:56:20 +0000
@@ -50,6 +50,7 @@
UI::EditBox edit_;
UI::Button ok_button_;
UI::Button cancel_button_;
+ const std::string illegal_filename_tooltip_;
void edit_changed();
};
=== modified file 'src/graphic/text_layout.cc'
--- src/graphic/text_layout.cc 2018-05-05 17:10:37 +0000
+++ src/graphic/text_layout.cc 2018-05-22 05:56:20 +0000
@@ -141,6 +141,13 @@
return f.str();
}
+/// Bullet list item
+std::string as_listitem(const std::string& txt, int ptsize, const RGBColor& clr) {
+ static boost::format f("<div width=100%%><div><p><font size=%d color=%s>•</font></p></div><div><p><space gap=6></p></div><div width=*><p><font size=%d color=%s>%s<vspace gap=6></font></p></div></div>");
+ f % ptsize % clr.hex_value() % ptsize % clr.hex_value() % txt;
+ return f.str();
+}
+
std::string as_richtext(const std::string& txt) {
static boost::format f("<rt>%s</rt>");
f % txt;
=== modified file 'src/graphic/text_layout.h'
--- src/graphic/text_layout.h 2018-04-07 16:59:00 +0000
+++ src/graphic/text_layout.h 2018-05-22 05:56:20 +0000
@@ -84,6 +84,10 @@
const RGBColor& clr = UI_FONT_CLR_FG,
UI::FontSet::Face face = UI::FontSet::Face::kSans);
+std::string as_listitem(const std::string&,
+ int ptsize = UI_FONT_SIZE_SMALL,
+ const RGBColor& clr = UI_FONT_CLR_FG);
+
std::string as_richtext(const std::string&);
std::string as_tooltip(const std::string&);
std::string as_waresinfo(const std::string&);
=== modified file 'src/io/CMakeLists.txt'
--- src/io/CMakeLists.txt 2016-02-06 11:11:24 +0000
+++ src/io/CMakeLists.txt 2018-05-22 05:56:20 +0000
@@ -23,24 +23,3 @@
io_filesystem
io_stream
)
-
-wl_library(io_filesystem
- SRCS
- filesystem/disk_filesystem.cc
- filesystem/disk_filesystem.h
- filesystem/filesystem.cc
- filesystem/filesystem.h
- filesystem/filesystem_exceptions.h
- filesystem/layered_filesystem.cc
- filesystem/layered_filesystem.h
- filesystem/zip_exceptions.h
- filesystem/zip_filesystem.cc
- filesystem/zip_filesystem.h
- DEPENDS
- base_exceptions
- base_log
- base_macros
- io_fileread
- io_stream
- third_party_minizip
-)
=== modified file 'src/io/filesystem/CMakeLists.txt'
--- src/io/filesystem/CMakeLists.txt 2012-02-15 21:25:34 +0000
+++ src/io/filesystem/CMakeLists.txt 2018-05-22 05:56:20 +0000
@@ -1,1 +1,24 @@
add_subdirectory(test)
+
+wl_library(io_filesystem
+ SRCS
+ disk_filesystem.cc
+ disk_filesystem.h
+ filesystem.cc
+ filesystem.h
+ filesystem_exceptions.h
+ layered_filesystem.cc
+ layered_filesystem.h
+ zip_exceptions.h
+ zip_filesystem.cc
+ zip_filesystem.h
+ DEPENDS
+ base_exceptions
+ base_i18n
+ base_log
+ base_macros
+ graphic_text_layout
+ io_fileread
+ io_stream
+ third_party_minizip
+)
=== modified file 'src/io/filesystem/filesystem.cc'
--- src/io/filesystem/filesystem.cc 2018-05-12 04:17:53 +0000
+++ src/io/filesystem/filesystem.cc 2018-05-22 05:56:20 +0000
@@ -43,8 +43,13 @@
#include <sys/stat.h>
#include <unistd.h>
+#include <boost/algorithm/string/predicate.hpp>
+#include <boost/format.hpp>
+
+#include "base/i18n.h"
#include "base/log.h"
#include "config.h"
+#include "graphic/text_layout.h"
#include "io/filesystem/disk_filesystem.h"
#include "io/filesystem/layered_filesystem.h"
#include "io/filesystem/zip_exceptions.h"
@@ -60,6 +65,34 @@
#define PATH_MAX MAX_PATH
#endif
+
+namespace {
+// Characters that are allowed in filenames, but not at the beginning
+const std::vector<std::string> illegal_filename_starting_characters{
+ ".",
+ "-",
+ " ", // Keep the blank last
+};
+
+// Characters that are disallowed anywhere in a filename
+// No potential file separators or other potentially illegal characters
+// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
+// http://www.linfo.org/file_name.html
+// https://support.apple.com/en-us/HT202808
+// We can't just regex for word & digit characters here because of non-Latin scripts.
+const std::vector<std::string> illegal_filename_characters{
+ "<",
+ ">",
+ ":",
+ "\"",
+ "|",
+ "?",
+ "*",
+ "/",
+ "\\",
+};
+} // namespace
+
/**
* \param path A file or directory name
* \return True if ref path is absolute and within this FileSystem, false otherwise
@@ -148,6 +181,43 @@
#endif
}
+bool FileSystem::is_legal_filename(const std::string& filename) {
+ if (filename.empty()) {
+ return false;
+ }
+ for (const std::string& illegal_start : illegal_filename_starting_characters) {
+ if (boost::starts_with(filename, illegal_start)) {
+ return false;
+ }
+ }
+ for (const std::string& illegal_char : illegal_filename_characters) {
+ if (boost::contains(filename, illegal_char)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+std::string FileSystem::illegal_filename_tooltip() {
+ std::vector<std::string> starting_characters;
+ for (const std::string& character : illegal_filename_starting_characters) {
+ if (character == " ") {
+ /** TRANSLATORS: Part of tooltip entry for characters in illegal filenames. replaces tha blank space in a list of illegal characters */
+ starting_characters.push_back(pgettext("illegal_filename_characters", "blank space"));
+ } else {
+ starting_characters.push_back(character);
+ }
+ }
+ /** TRANSLATORS: Tooltip entry for characters in illegal filenames. %s is a list of illegal characters */
+ const std::string illegal_start(as_listitem((boost::format(pgettext("illegal_filename_characters", "%s at the start of the filename")) % richtext_escape(i18n::localize_list(starting_characters, i18n::ConcatenateWith::OR))).str(), UI_FONT_SIZE_MESSAGE));
+
+ /** TRANSLATORS: Tooltip entry for characters in illegal filenames. %s is a list of illegal characters */
+ const std::string illegal(as_listitem((boost::format(pgettext("illegal_filename_characters", "%s anywhere in the filename")) % richtext_escape(i18n::localize_list(illegal_filename_characters, i18n::ConcatenateWith::OR))).str(), UI_FONT_SIZE_MESSAGE));
+
+ /** TRANSLATORS: Tooltip header for characters in illegal filenames. This is followed by a list of bullet points */
+ return (boost::format("%s%s%s") % pgettext("illegal_filename_characters", "The following characters are not allowed:") % illegal_start % illegal).str();
+}
+
// TODO(unknown): Write homedir detection for non-getenv-systems
std::string FileSystem::get_homedir() {
std::string homedir;
=== modified file 'src/io/filesystem/filesystem.h'
--- src/io/filesystem/filesystem.h 2018-04-07 16:59:00 +0000
+++ src/io/filesystem/filesystem.h 2018-05-22 05:56:20 +0000
@@ -108,6 +108,10 @@
std::string canonicalize_name(std::string path) const;
bool is_path_absolute(const std::string& path) const;
+ /// Returns true if the filename is legal in all operating systems
+ static bool is_legal_filename(const std::string& filename);
+ static std::string illegal_filename_tooltip();
+
// Returns the path separator, i.e. \ on windows and / everywhere else.
static char file_separator();
=== modified file 'src/io/filesystem/layered_filesystem.cc'
--- src/io/filesystem/layered_filesystem.cc 2018-04-07 16:59:00 +0000
+++ src/io/filesystem/layered_filesystem.cc 2018-05-22 05:56:20 +0000
@@ -22,16 +22,12 @@
#include <cstdio>
#include <memory>
-#include <boost/algorithm/string/predicate.hpp>
-#include <boost/regex.hpp>
-
#include "base/log.h"
#include "base/wexception.h"
#include "io/fileread.h"
#include "io/streamread.h"
LayeredFileSystem* g_fs;
-
LayeredFileSystem::LayeredFileSystem() : home_(nullptr) {
}
@@ -41,18 +37,6 @@
LayeredFileSystem::~LayeredFileSystem() {
}
-bool LayeredFileSystem::is_legal_filename(const std::string& filename) {
- // No potential file separators or other potentially illegal characters
- // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
- // http://www.linfo.org/file_name.html
- // https://support.apple.com/en-us/HT202808
- // We can't just regex for word & digit characters here because of non-Latin scripts.
- boost::regex re(".*[<>:\"|?*/\\\\].*");
- return !filename.empty() && !boost::starts_with(filename, ".") &&
- !boost::starts_with(filename, " ") && !boost::starts_with(filename, "-") &&
- !boost::regex_match(filename, re);
-}
-
/**
* Just assume that at least one of our child FSs is writable
*/
=== modified file 'src/io/filesystem/layered_filesystem.h'
--- src/io/filesystem/layered_filesystem.h 2018-04-07 16:59:00 +0000
+++ src/io/filesystem/layered_filesystem.h 2018-05-22 05:56:20 +0000
@@ -58,8 +58,7 @@
std::set<std::string> list_directory(const std::string& path) override;
- /// Returns true if the filename is legal in all operating systems
- static bool is_legal_filename(const std::string& filename);
+
bool is_writable() const override;
bool file_exists(const std::string& path) override;
bool is_directory(const std::string& path) override;
=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc 2018-05-14 06:59:39 +0000
+++ src/ui_basic/panel.cc 2018-05-22 05:56:20 +0000
@@ -143,8 +143,9 @@
app->set_mouse_lock(false); // more paranoia :-)
Panel* forefather = this;
- while (Panel* const p = forefather->parent_)
- forefather = p;
+ while (forefather->parent_ != nullptr) {
+ forefather = forefather->parent_;
+ }
default_cursor_ = g_gr->images().get("images/ui_basic/cursor.png");
default_cursor_click_ = g_gr->images().get("images/ui_basic/cursor_click.png");
@@ -175,13 +176,16 @@
app->handle_input(&input_callback);
if (start_time >= next_think_time) {
- if (app->should_die())
+ if (app->should_die()) {
end_modal<Returncodes>(Returncodes::kBack);
+ }
do_think();
- if (flags_ & pf_child_die)
+ if (flags_ & pf_child_die) {
check_child_death();
+ }
+
next_think_time = start_time + kGameLogicDelay;
}
@@ -190,7 +194,13 @@
forefather->do_draw(rt);
rt.blit((app->get_mouse_position() - Vector2i(3, 7)),
app->is_mouse_pressed() ? default_cursor_click_ : default_cursor_);
- forefather->do_tooltip();
+
+ if (is_modal()) {
+ do_tooltip();
+ } else {
+ forefather->do_tooltip();
+ }
+
g_gr->refresh();
next_draw_time = start_time + draw_delay;
}
@@ -594,8 +604,7 @@
* false otherwise.
*/
bool Panel::handle_tooltip() {
- RenderTarget& rt = *g_gr->get_render_target();
- return draw_tooltip(rt, tooltip());
+ return draw_tooltip(tooltip());
}
/**
@@ -1060,13 +1069,15 @@
/**
* Draw the tooltip. Return true on success
*/
-bool Panel::draw_tooltip(RenderTarget& dst, const std::string& text) {
+bool Panel::draw_tooltip(const std::string& text) {
if (text.empty()) {
return false;
}
+
+ RenderTarget& dst = *g_gr->get_render_target();
std::string text_to_render = text;
if (!is_richtext(text_to_render)) {
- text_to_render = as_tooltip(text);
+ text_to_render = as_tooltip(text_to_render);
}
constexpr uint32_t kTipWidthMax = 360;
=== modified file 'src/ui_basic/panel.h'
--- src/ui_basic/panel.h 2018-04-27 06:11:05 +0000
+++ src/ui_basic/panel.h 2018-05-22 05:56:20 +0000
@@ -311,7 +311,7 @@
static void play_new_chat_member();
static void play_new_chat_message();
- static bool draw_tooltip(RenderTarget&, const std::string& text);
+ static bool draw_tooltip(const std::string& text);
void draw_background(RenderTarget& dst, const UI::PanelStyleInfo&);
void draw_background(RenderTarget& dst, Recti rect, const UI::PanelStyleInfo&);
=== modified file 'src/wui/game_main_menu_save_game.cc'
--- src/wui/game_main_menu_save_game.cc 2018-05-13 07:15:39 +0000
+++ src/wui/game_main_menu_save_game.cc 2018-05-22 05:56:20 +0000
@@ -65,7 +65,8 @@
cancel_(&buttons_box_, "cancel", 0, 0, 0, 0, UI::ButtonStyle::kWuiSecondary, _("Cancel")),
ok_(&buttons_box_, "ok", 0, 0, 0, 0, UI::ButtonStyle::kWuiPrimary, _("OK")),
- curdir_(kSaveDir) {
+ curdir_(kSaveDir),
+ illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) {
layout();
@@ -130,8 +131,9 @@
void GameMainMenuSaveGame::edit_box_changed() {
// Prevent the user from creating nonsense directory names, like e.g. ".." or "...".
- const bool is_legal_filename = LayeredFileSystem::is_legal_filename(filename_editbox_.text());
+ const bool is_legal_filename = FileSystem::is_legal_filename(filename_editbox_.text());
ok_.set_enabled(is_legal_filename);
+ filename_editbox_.set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_);
load_or_save_.delete_button()->set_enabled(false);
load_or_save_.clear_selections();
}
=== modified file 'src/wui/game_main_menu_save_game.h'
--- src/wui/game_main_menu_save_game.h 2018-04-07 16:59:00 +0000
+++ src/wui/game_main_menu_save_game.h 2018-05-22 05:56:20 +0000
@@ -80,6 +80,7 @@
std::string parentdir_;
std::string filename_;
bool overwrite_;
+ const std::string illegal_filename_tooltip_;
};
#endif // end of include guard: WL_WUI_GAME_MAIN_MENU_SAVE_GAME_H
Follow ups
