[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB361412: [EditLine] Rewrite GetHistoryFilePath (authored by JDevlieghere, committed by ). Changed prior to commit: https://reviews.llvm.org/D62216?vs=200603&id=200791#toc Repository: rLLDB LLDB CH

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. > Also, when we say "no history", we mean "no persistent history", right? I'd > expect one would still be able to use the history of commands typed in the > current session in this scenario... Yup! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-22 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good to me. In D62216#1511101 , @JDevlieghere wrote: > In D62216#1511092 , @jingham wrote: > > > Most o

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 200603. JDevlieghere added a comment. Add comment describing lldb's behavior regarding history files CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/ https://reviews.llvm.org/D62216 Files: lldb/source/Host/common/Editline.cpp Index

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Then put in a comment saying something like "LLDB ONLY stores history files in .lldb and if you don't like that..." If you are instituting a policy which is not common you should at least document it... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In D62216#1511092 , @jingham wrote: > Most other programs write their history files in ~. So we are being a little > odd in offering to put them in ~/.lldb, though I agree that is convenient. > > But if putting files in ~/.l

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Most other programs write their history files in ~. So we are being a little odd in offering to put them in ~/.lldb, though I agree that is convenient. But if putting files in ~/.lldb ticked somebody off enough that they made a .lldb directory that was read only, your

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. What if ~ is not writable either? I think we shouldn't fallback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/ https://reviews.llvm.org/D62216 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. I figured if we're changing this anyway, we could as well change the behavior. I think it's reasonable to not have history when we cannot create the `.lldb` directory. Before I wiped my directory, I saw several (5?) files in `.lldb` and I'm not convinced we should

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I don't think that's quite right. I think the behavior should be: 1. Try to make a .lldb directory a) If you can, then i) if wchar support ~/.lldb/.lldb-widehistory ii) else ~/.lldb/lldb-history b) else i) if wchar support ~/.lldb-widehistory ii) else ~/.lldb-history

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 200592. JDevlieghere added a comment. Jim's feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/ https://reviews.llvm.org/D62216 Files: lldb/source/Host/common/Editline.cpp Index: lldb/source/Host/common/Editline.cpp =

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Why are we calling this "widehistory" because we couldn't write into the .lldb directory? I know that's the way it was but it makes no sense. I guess it would make sense to call it widehistory if edit line was in wchar mode, that way you wouldn't ask a non wchar edit

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

2019-05-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision. JDevlieghere added reviewers: labath, xiaobai, jingham. Herald added a project: LLDB. Rewrite the GetHistoryFilePath implementation without relying on FileSpec in the spirit of our discussion in D61994 . Repository: rLLDB LL