Display current project name in main window #7789

le717 · 2014-05-08T20:38:51Z

Supersedes #6878.

This implements #2281, displaying the project name in the Brackets window title.

marcelgerber · 2014-05-10T20:32:14Z

@le717 This code is better:

    $(ProjectManager).on("projectOpen", function (e, projectRoot) {
        _projectName = projectRoot.name;
        updateTitle();
    });

le717 · 2014-05-10T20:58:23Z

@SAplayer That worked! After looking more closely at it, I found out _projectName was not getting updated even though the project name was being retrieved. I didn't think about calling updateTitle() after getting it (although that makes sense). Thanks!

The only thing left now would be this split-second display of null on load.

However, because it appears for such a short time, it might be acceptable.

@peterflynn Any chance this could get in Release 40?

marcelgerber · 2014-05-23T19:14:00Z

I guess initializing _projectName = ""; would be a little bit more subtle, but the perfect solution is, of course, making the title context-specific.

You could probably try this:

_projectName = _projectName.replace(/^\s*(-|\u2014)/, "").trim();

le717 · 2014-05-23T20:48:00Z

@SAplayer I tried setting it to an empty string, but then I got that blank dash in front of Brackets (which you regex would fix). I'm just trying to think where the regex would go in the code if the _projectName was initialized as an empty string. I'm thinking right before the project name is set?

marcelgerber · 2014-05-23T21:01:19Z

Probably like this:

    $(ProjectManager).on("projectOpen", function (e, projectRoot) {
        _projectName = projectRoot.name;
        _projectName = _projectName.replace(/^\s*[\-\u2014]\s*/, "");
        updateTitle();
    });

marcelgerber · 2014-05-24T09:47:10Z

Forget what I said.

if (_projectName === "") {
  windowTitle = windowTitle.replace(/^\s*[\-\u2014]\s*/, "");
}
window.document.title = windowTitle;

Tested.

le717 · 2014-05-24T10:20:35Z

@SAplayer Oh, come on! I had that exact same code (save the regex) in place before, but I removed it long before I committed (wasn't sure if it was what @peterflynn was talking about). I didn't think that code snippet from last night was what was needed, but I know this new one would work even is you had not tested it. I'll make the change ASAP (on mobile right now).

marcelgerber · 2014-05-24T18:24:49Z

src/document/DocumentCommandHandlers.js

-    /** @type {string} String template for window title. Use emdash on mac only. */
-    var WINDOW_TITLE_STRING = (brackets.platform !== "mac") ? "{0} - {1}" : "{0} \u2014 {1}";
+    /** @type {string} The current project name; displayed in window title */
+    var _projectName = null;


Set it to "" in order to make the code below work.

*facepalm* Fixed.

le717 · 2014-05-24T19:00:27Z

This PR should (hopefully) finally be complete, with all tests passing. @peterflynn Ready for review again.

Thanks for all the help, @SAplayer. I probably wouldn't have been able to finish this until later if it were not for you. :)

redmunds · 2014-06-03T22:56:53Z

Changed Milestone to Release 41.

le717 · 2014-07-19T23:43:12Z

@peterflynn Do you still want to review this? I just rebased it and merged with master, so it's all ready to pull. If I need, I will try to fix the whitespace changes. That was from a force-of-habit keyboard shortcut action when I started this.

le717 · 2014-08-22T17:57:45Z

@peterflynn Is this feature still wanted in Brackets? I can post a quick screenshot of the final version if needed.

ingorichter · 2014-11-18T19:45:37Z

@le717 Would you mind updating the screenshot? That would be great.
@peterflynn I guess we still want to have this feature added, correct?

le717 · 2014-11-18T20:04:51Z

@ingorichter Quick check, what screenshot do you refer to? Yes, I'll get this PR ready to be merged again, no problem there.

le717 · 2014-11-18T20:29:30Z

@ingorichter

With working file

Without working file

Is that what you meant?

EDIT: Updated, ready to be merged, Travis build is clean.

ingorichter · 2014-11-19T17:51:00Z

src/document/DocumentCommandHandlers.js

+    * String template for window title when a file is open. Use emdash on mac only.
+    * @type {string}
+    */
+    var WINDOW_TITLE_STRING_DOC = (brackets.platform !== "mac") ? "{0} ({1}) - {2}" : "{0} \u2014 {2}";


I think the new format should be available for the Mac too. Currently it's missing and the tests are failing.

I'd do OS differentiation like this:

var osDash = brackets.platform === "mac" ? "\u2014" : "-"; var WINDOW_TITLE_STRING_NO_DOC = "{0} " + osDash + " {1}"; var WINDOW_TITLE_STRING_DOC = "{0} ({1}) " + osDash + " {2}";

Oh shoot, I didn't realize this was not occurring on Mac.
Before I update, my question would be why different dashes are used on Win and Mac. OS app convention/requirement? Why not use the same dash on Win, Mac, and Linux?

I don't know. But what I heart is, that \2014 is the default delimiter on OSX and - for all other platforms.

I don't see why we can't use \2014 on all platforms then. It's just a tad longer than -. *sigh* Will fix.

@le717 We decided originally we want to match each platform's standard separator, so can you please keep the split? \u2014 looks weird on Windows (doesn't match any other app), and - looks weird on Mac (same reason). We want to feel as much like native code editors as possible.

Fully understand that native app feeling. Just trying to understand the reasoning behind the difference. I was going to keep the split dashes anyway.

peterflynn · 2014-11-19T20:54:57Z

src/document/DocumentCommandHandlers.js

@@ -135,7 +147,7 @@ define(function (require, exports, module) {
    function _updateTitle() {
        var currentDoc = DocumentManager.getCurrentDocument(),
            currentlyViewedPath = MainViewManager.getCurrentlyViewedPath(MainViewManager.ACTIVE_PANE),
-            windowTitle = brackets.config.app_title;
+            windowTitle = StringUtils.format(WINDOW_TITLE_STRING_NO_DOC, _projectName, brackets.config.app_title);


It'd be cleaner to move this initializer down into an else with the other case below.

le717 · 2014-11-20T20:05:49Z

@ingorichter @peterflynn @marcelgerber Changes pushed. Project title should now work on Mac (with appropriate dashes), refactoring completed, and null issues I keep hitting should be fixed now.

peterflynn · 2014-11-21T18:39:34Z

src/document/DocumentCommandHandlers.js

        } else {
-            // hide dirty dot if there is no document
-            _$dirtydot.css("visibility", "hidden");
+            // Application loading/no project


We can never get here -- if ProjectManager.getProjectRoot() is null, we'll crash up on line 149 before getting this far. We should either remove this else or move the projectName var down inside the if (projectRoot) block here.

I'm in favor of moving it into if (projectRoot).

I never ended up in the else branch. Perhaps you can initialize windowTitle with brackets.config.app_title before the if (projectRoot) statement and get rid of the else branch entirely (only to initialize it with something valid).

@ingorichter Sounds good. It looks like it was that way in case projectRoot was undefined or null, but initializing it as brackets.config.app_title is a better method.

le717 · 2014-11-21T19:02:17Z

@peterflynn Changes pushed.

ingorichter · 2014-11-21T21:29:11Z

src/document/DocumentCommandHandlers.js

-            windowTitle = (currentDoc.isDirty) ? "• " + windowTitle : windowTitle;
+        var projectRoot = ProjectManager.getProjectRoot();
+        if (projectRoot) {
+            var projectName = ProjectManager.getProjectRoot().name;


nit: you can reuse projectRoot here instead of calling ProjectManager.getProjectRoot again.

marcelgerber · 2014-11-21T21:34:11Z

JSHint:

Linting test/spec/DocumentCommandHandlers-test.js...ERROR
[L44:C20] W098: 'StringUtils' is defined but never used.
        StringUtils              = require("utils/StringUtils");

le717 · 2014-11-21T21:36:49Z

@marcelgerber Fixed just as you posted that.

le717 · 2014-11-21T21:42:16Z

Changes pushed, ready for another look.

peterflynn · 2014-11-25T21:15:06Z

@le717 Just needs a merge with master so you're in sync with the new way of registering event listeners, and then I think this is good to go!

And actually if you could squash/rebase after doing your merge with master, that would be ideal -- there are a lot of commits here and many of them are redundant merge commits.

le717 · 2014-11-25T21:25:57Z

@peterflynn Certainly for both merge and squash. Will get that ready ASAP. :)

le717 · 2014-11-25T22:09:39Z

@peterflynn Merged and rebased. Travis is running right now. Unless it reports something, this is ready to be merged. :D

peterflynn · 2014-11-25T22:46:34Z

Great, all set! Thanks for the typo/spelling fixes too btw :-)

Display current project name in window titlebar

le717 · 2014-11-25T22:52:01Z

redmunds · 2014-12-09T17:50:24Z

src/document/DocumentCommandHandlers.js

@@ -194,7 +208,7 @@ define(function (require, exports, module) {
     */
    function _shortTitleForDocument(doc) {
        var fullPath = doc.file.fullPath;
-        
+


Argh! All of the unnecessary whitespace changes in this file (and others) are making my life difficult. I'm trying to keep my promises-upgrade branch up-to-date and every one of these whitespace changes that coincide with any change in my branch causes a phantom merge conflict that needs to be manually resolved. Please stop doing this! Brackets has no convention for whitespace in empty lines -- it's left to the authors discretion to have none or match indent level of previous/next line of code, so there's nothing to "fix".

Sorry @le717 I don't mean to single you out -- I'm just using your code as an example. This comment is meant for everyone: contributors and committers.

I felt bad about these whitespaces things when this was finally merged. This was my first PR, and the whitespace was removed automatically. I kept considering adding it back, but it was mentioned or asked for. If someone had asked for it to be restored, I would have gladly done so. I've been trying to watch whitespace removal since this.

It's fine, I know how it is to be frustrated by changes like this. ;)

The team has gone back and forth about whether to accept these changes, but I think this is a good reason to no longer accept them.

@redmunds Pro Tip: Use git merge -X ignore-all-space
Source: http://stackoverflow.com/a/9784089/3455922 (haven't tested it, though)

@marcelgerber Thanks for the tip! I'll try to remember that one.

le717 mentioned this pull request May 8, 2014

Display current project title in main window #6878

Closed

njx assigned peterflynn May 9, 2014

njx added the low priority label May 9, 2014

peterflynn added this to the Release #40 milestone May 14, 2014

le717 changed the title ~~Display current project title in main window~~ Display current project name in main window May 23, 2014

marcelgerber reviewed May 24, 2014
View reviewed changes

redmunds modified the milestones: Release #41, Release #40 Jun 3, 2014

JeffryBooher removed this from the Release 0.41 milestone Jun 20, 2014

ingorichter reviewed Nov 19, 2014
View reviewed changes

ingorichter added the Review label Nov 19, 2014

ingorichter assigned ingorichter and unassigned peterflynn Nov 19, 2014

peterflynn reviewed Nov 19, 2014
View reviewed changes

peterflynn reviewed Nov 21, 2014
View reviewed changes

ingorichter reviewed Nov 21, 2014
View reviewed changes

Issue #2281, display project name in window title

8ba112c

peterflynn assigned peterflynn and unassigned ingorichter Nov 25, 2014

peterflynn added a commit that referenced this pull request Nov 25, 2014

Merge pull request #7789 from le717/project-name-in-title

b8bf738

Display current project name in window titlebar

peterflynn merged commit b8bf738 into adobe:master Nov 25, 2014

peterflynn removed the Review label Nov 25, 2014

le717 deleted the project-name-in-title branch November 25, 2014 22:53

redmunds reviewed Dec 9, 2014
View reviewed changes

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] Display current project name in main window brackets-archive/bracketsIssues#6987

Open

Display current project name in main window #7789

Display current project name in main window #7789

Conversation

le717 commented May 8, 2014

marcelgerber commented May 10, 2014

le717 commented May 10, 2014

marcelgerber commented May 23, 2014

le717 commented May 23, 2014

marcelgerber commented May 23, 2014

marcelgerber commented May 24, 2014

le717 commented May 24, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

le717 commented May 24, 2014

redmunds commented Jun 3, 2014

le717 commented Jul 19, 2014

le717 commented Aug 22, 2014

ingorichter commented Nov 18, 2014

le717 commented Nov 18, 2014

le717 commented Nov 18, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

le717 commented Nov 20, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

le717 commented Nov 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcelgerber commented Nov 21, 2014

le717 commented Nov 21, 2014

le717 commented Nov 21, 2014

peterflynn commented Nov 25, 2014

le717 commented Nov 25, 2014

le717 commented Nov 25, 2014

peterflynn commented Nov 25, 2014

le717 commented Nov 25, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment