D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham abandoned this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Nathaniel Graham]

ngraham added a comment.


  I've submitted a proposed replacement for this as D21907: Show feedback 
inline when creating new files or folders .

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Nathaniel Graham]

ngraham added a comment.


  Yes, I plan to put it up for review today or tomorrow. Sorry for the long 
wait!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @ngraham Are you still working on this?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @ngraham Any updates?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Nathaniel Graham]

ngraham added a comment.


  Yes, I think people are correct that these modal dialogs are annoying. I'm 
working on something to show the errors and warnings inline at the moment when 
you're typing the file/folder name in the first place.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @ngraham I think this patch is behaving exactly it should be, are you trying 
for a fix so that these warnings are not used?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Nathaniel Graham]

ngraham added a comment.


  I'm working on an alternative user interface for this. Stay tuned!

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  Ping? Can someone please review?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Emirald Mateli]

emateli added a comment.


  I am of the opinion that such dialogs are not very user friendly and honestly 
very much against them. Not commenting on the usefulness of this particular 
warning, but dolphin has quite a few of these kind of dialogs with the same 
process: `Input something -> Get error -> Restart`. Feedback should instead be 
given inplace.
  
  A new dialog built especially for Dolphin's new file/folder/X should take 
over getting the input from the user and prevent continuing or warn the user 
about his actions:
  
  - User leaves empty input? "Name cannot be empty".
  - User types existing folder name? Show him an error message on the spot
  - User types "~" as part of the file name? Show this warning
  - User types names with `/`, warn that a path will be created instead of just 
a single directory.
  - User types a name that starts with a dot? Show him the current warning 
about hidden files etc.
  
  I am strongly of the opinion that such dialog is vastly more user friendly to 
what we currently have and want to implement. Pinging @ngraham since he's 
particularly concerned with UX/Usability these days. Looking forward to 
everyone's thoughts.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: emateli, pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  Ping?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham retitled this revision from "Create directory named '~' and throw a 
warning before creating it successfully." to "Allow creating directory named 
'~' and throw a warning before creating it.".

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham updated this revision to Diff 49943.
shubham marked an inline comment as done.
shubham added a comment.


  Remove redundant setting of window title.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18384?vs=49935=49943

BRANCH
  dir

REVISION DETAIL
  https://phabricator.kde.org/D18384

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp
  src/filewidgets/knewfilemenu.h

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Pino Toscano]

pino added a comment.


  Ah yes, now I see it better, the whole KMessageBox::shouldBeShownContinue() 
check is bogus, since that key is not set by anything.

INLINE COMMENTS

> knewfilemenu.cpp:422
>  QDialog *confirmDialog = new QDialog(m_parentWidget);
>  confirmDialog->setWindowTitle(i18n("Create hidden directory?"));
>  confirmDialog->setModal(m_modal);

this is redundant now, as done in both the if branches below

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @pino The test case you told is not applicable(I confirmed).
  My test case:
  
  1. Create directory named .foo (Results in a warning)
  2. User checks "Do not ask again".
  3. User tries to create directory named .xyz (This time NO WARNING appears, 
as it is obvious)
  4. User tries to create directory named ~ (A warning is shown)
  
  So the conclusion is that the 2 warning messageboxes are treated 
individually, and their key is also set individually.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Pino Toscano]

pino added a comment.


  In D18384#396895 , @shubham wrote:
  
  > @pino got it what you meant.
  
  
  Definitely not. Let me explain it again:
  
  - so far, KNewFileMenuPrivate::confirmCreatingHiddenDir is used only to ask 
to the user whether choice of a directory starting with '.' is wanted; 
KNewFileMenuPrivate::confirmCreatingHiddenDir checks whether ask by using the 
message box "do not ask again" key "confirm_create_hidden_tilde_dir"
  - a user tries to create a directory starting with '.':
1. they get the messagebox that confirmCreatingHiddenDir shows
2. they tick the "do not ask again"
3. they proceed
  - as result of the point above, the key "confirm_create_hidden_tilde_dir" is 
set
  - your changes rename KNewFileMenuPrivate::confirmCreatingHiddenDir to 
confirmCreatingDir, and make it used also when a directory starts with '~'
  - the same user tries to create a directory starting with '~':
1. confirmCreatingDir is called
2. the "confirm_create_hidden_tilde_dir" key is set, so no message box is 
shown
3. the directory is created directly, without user confirmation

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham updated this revision to Diff 49935.
shubham added a comment.


  1. Use ==
  2. Rename slot's name

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18384?vs=49914=49935

BRANCH
  dir

REVISION DETAIL
  https://phabricator.kde.org/D18384

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp
  src/filewidgets/knewfilemenu.h

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham marked 3 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @pino got it what you meant. Also do I need to change the  
_k_slotCreateHiddenDirectory( ) slot name?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> shubham wrote in knewfilemenu.cpp:439
>   == 
> 
> internally calls
> 
>   operator==()

The compiler automatically replaces the operators into the calls to the 
appropriate functions. There is no speed penalty.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added a comment.


  @pino Sorry for inconvenience caused. BTW I did not get what you meant?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added inline comments.

INLINE COMMENTS

> dhaumann wrote in knewfilemenu.cpp:439
> Really? But isn't == not exactly the same? Can you elaborate?

== 

internally calls

  operator==()

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Pino Toscano]

pino added a comment.


  Also, not related to the code: @shubham, you seem to often remove your own 
comments. This is a bad practice for many POV of views (transparency, breaks 
the logic of a conversation, etc). As these reviews send notification emails to 
mailing lists usually, then your removed messages are archived, and thus 
removing them is useless.
  Please stop doing this anti-social practice, thank you.

INLINE COMMENTS

> knewfilemenu.cpp:408-413
> +void KNewFileMenuPrivate::confirmCreatingDir(const QString )
>  {
>  if 
> (!KMessageBox::shouldBeShownContinue(QStringLiteral("confirm_create_hidden_dir")))
>  {
>  _k_slotCreateHiddenDirectory();
>  return;
>  }

renaming confirmCreatingHiddenDir and using it also for another case means that 
the "confirm_create_hidden_dir" messagebox confirmation also applies to the 
other cases (filename == '~' in this case)

> knewfilemenu.cpp:911
>  }
> +
>  url = baseUrl;

extra line added

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Dominik Haumann]

dhaumann added inline comments.

INLINE COMMENTS

> shubham wrote in knewfilemenu.cpp:439
> This is faster than simple ==

Really? But isn't == not exactly the same? Can you elaborate?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Shubham]

shubham added inline comments.

INLINE COMMENTS

> dhaumann wrote in knewfilemenu.cpp:439
> Please simply write name == QStringLiteral (...) instead of operator==. Same 
> below.

This is faster than simple ==

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D18384: Allow creating directory named '~' and throw a warning before creating it.

[Dominik Haumann]

dhaumann added inline comments.

INLINE COMMENTS

> knewfilemenu.cpp:439
> KMessageBox::NoExec);
> +} else if (name.operator==(QLatin1String("~"))) {
> +confirmDialog->setWindowTitle(i18n("Create directory named ~?"));

Please simply write name == QStringLiteral (...) instead of operator==. Same 
below.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D18384

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns