Permalink
Browse files

Implementation of monitor/unmonitor command line APIs (only stack fra…

…mes are missing)
  • Loading branch information...
1 parent 50240f1 commit 2c5d57549eefb2c9f9184cc6cc84acdaed382fbc @janodvarko janodvarko committed Mar 27, 2013
View
152 extension/content/firebug/console/functionMonitor.js
@@ -48,16 +48,64 @@ var FunctionMonitor = Obj.extend(Firebug.Module,
Firebug.Module.shutdown.apply(this, arguments);
},
+ initContext: function(context)
+ {
+ var tool = context.getTool("debugger");
+ tool.addListener(this);
+ },
+
+ destroyContext: function(context)
+ {
+ var tool = context.getTool("debugger");
+ tool.removeListener(this);
+ },
+
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
- // Firebug.Debugger listener
+ // DebuggerTool Listener
+
+ onDebuggerPaused: function(context, event, packet)
+ {
+ // The function monitor is only interested in 'breakpoint' type of interrupts.
+ var type = packet.why.type;
+ if (type != "breakpoint")
+ return false;
+
+ var frame = context.stoppedFrame;
+ var bp = BreakpointStore.findBreakpoint(frame.href, frame.line - 1,
+ BreakpointStore.BP_MONITOR);
+
+ Trace.sysout("functionMonitor.onDebuggerPaused; " + frame.href + " (" +
+ frame.line + ") " + (bp ? "monitor exists" : "no monitor"), packet);
+
+ if (!bp)
+ return false;
+
+ // Log into the Console panel if there is a monitor.
+ this.onMonitorScript(context, frame);
+
+ // Let's see if there is also a standard breakpoint. If yes, make sure the
+ // debugger breaks (by returning false).
+ bp = BreakpointStore.findBreakpoint(frame.href, frame.line - 1);
+ if (bp)
+ {
+ Trace.sysout("functionMonitor.onDebuggerPaused; There is also a normal BP", bp);
+ return false;
+ }
+
+ // ... otherwise do not break and just resume the debugger.
+ return true;
+ },
onMonitorScript: function(context, frame)
{
- var stackTrace = StackFrame.buildStackTrace(frame);
+ Trace.sysout("functionMonitor.onMonitorScript;", frame);
+
+ // xxxHonza: how to get the current stack trace?
+ var stackTrace = null;//StackFrame.buildStackTrace(frame);
Firebug.Console.log(new FunctionLog(frame, stackTrace), context);
},
- onFunctionCall: function(context, frame, depth, calling)
+ /*onFunctionCall: function(context, frame, depth, calling)
{
//var url = Url.normalizeURL(frame.script.fileName);
//var sourceFile = context.sourceFileMap[url];
@@ -73,7 +121,7 @@ var FunctionMonitor = Obj.extend(Firebug.Module,
Firebug.Console.openGroup([frame, "depth:" + depth], context);
else
Firebug.Console.closeGroup(context);
- },
+ },*/
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
// Debugging and monitoring
@@ -120,26 +168,22 @@ var FunctionMonitor = Obj.extend(Firebug.Module,
Trace.sysout("functionMonitor.monitorScript; " + script.url + ", " +
script.startLine, fn);
- if (mode == "debug")
- {
- var location = {line: script.startLine, url: script.url};
-
- // If the first line of the script contains no code, slide down to
- // the nextline the has runnable code.
- location = DebuggerLib.getNextExecutableLine(context, location);
-
- // Create a new breakpoint.
- tool.setBreakpoint(context, location.url, location.line - 1,
- function(response, bpClient)
- {
- BreakpointStore.addBreakpoint(bpClient.location.url,
- bpClient.location.line - 1);
- });
- }
- else if (mode == "monitor")
+ var location = {line: script.startLine, url: script.url};
+
+ // If the first line of the script contains no code, slide down to
+ // the nextline the has runnable code.
+ location = DebuggerLib.getNextExecutableLine(context, location);
+
+ // Create a new breakpoint.
+ tool.setBreakpoint(context, location.url, location.line - 1,
+ function(response, bpClient)
{
- this.monitor(context, scriptInfo.sourceFile, scriptInfo.lineNo, Firebug.Debugger);
- }
+ var type = (mode == "monitor") ? BreakpointStore.BP_MONITOR :
+ BreakpointStore.BP_NORMAL;
+
+ BreakpointStore.addBreakpoint(bpClient.location.url,
+ bpClient.location.line - 1, type);
+ });
}
},
@@ -152,48 +196,22 @@ var FunctionMonitor = Obj.extend(Firebug.Module,
Trace.sysout("functionMonitor.unmonitorScript; " + script.url + ", " +
script.startLine, fn);
- if (mode == "debug")
- {
- var location = {line: script.startLine, url: script.url};
- location = DebuggerLib.getNextExecutableLine(context, location);
- BreakpointStore.removeBreakpoint(location.url, location.line - 1);
- }
- else if (mode == "monitor")
- {
- this.unmonitor(context, scriptInfo.sourceFile.href, scriptInfo.lineNo);
- }
- }
- },
-
- monitor: function(context, sourceFile, lineNo, debuggr)
- {
- // xxxHonza
- return;
+ var location = {line: script.startLine, url: script.url};
+ location = DebuggerLib.getNextExecutableLine(context, location);
- if (lineNo == -1)
- return null;
+ var type = (mode == "monitor") ? BreakpointStore.BP_MONITOR :
+ BreakpointStore.BP_NORMAL;
- var bp = this.addBreakpoint(BP_MONITOR, sourceFile, lineNo, null, debuggr);
- if (bp)
- {
- ++monitorCount;
- dispatch(debuggers, "onToggleMonitor", [sourceFile.href, lineNo, true]);
+ BreakpointStore.removeBreakpoint(location.url, location.line - 1, type);
}
-
- return bp;
},
- unmonitor: function(href, lineNo)
+ // xxxHonza: this will be needed for the context menu.
+ isMonitored: function(url, linNo)
{
- // xxxHonza
- return;
-
- if (lineNo != -1 && this.removeBreakpoint(BP_MONITOR, href, lineNo))
- {
- --monitorCount;
- dispatch(debuggers, "onToggleMonitor", [ href, lineNo, false]);
- }
- },
+ var bp = lineNo != -1 ? BreakpointStore.findBreakpoint(url, lineNo) : null;
+ return bp && bp.type & BreakpointStore.BP_MONITOR;
+ }
});
// ********************************************************************************************* //
@@ -239,7 +257,7 @@ var FunctionMonitorRep = domplate(Firebug.Rep,
hasStackTrace: function(object)
{
- return true;
+ return !!object.stackTrace;
},
getTitle: function(object)
@@ -318,32 +336,36 @@ var FunctionMonitorRep = domplate(Firebug.Rep,
function debug(context, args)
{
var fn = args[0];
-
FunctionMonitor.monitorFunction(context, fn, "debug");
+ var msg = Locale.$STRF("functionMonitor.Breakpoint_created", [fn.name]);
+ Firebug.Console.logFormatted([msg], context, "info");
return Firebug.Console.getDefaultReturnValue(context.window);
}
function undebug(context, args)
{
var fn = args[0];
-
FunctionMonitor.unmonitorFunction(context, fn, "debug");
+ var msg = Locale.$STRF("functionMonitor.Breakpoint_removed", [fn.name]);
+ Firebug.Console.logFormatted([msg], context, "info");
return Firebug.Console.getDefaultReturnValue(context.window);
}
function monitor(context, args)
{
var fn = args[0];
-
FunctionMonitor.monitorFunction(context, fn, "monitor");
+ var msg = Locale.$STRF("functionMonitor.Monitor_created", [fn.name]);
+ Firebug.Console.logFormatted([msg], context, "info");
return Firebug.Console.getDefaultReturnValue(context.window);
}
-function unmonitor(fn)
+function unmonitor(context, args)
{
var fn = args[0];
-
FunctionMonitor.unmonitorFunction(context, fn, "monitor");
+ var msg = Locale.$STRF("functionMonitor.Monitor_removed", [fn.name]);
+ Firebug.Console.logFormatted([msg], context, "info");
return Firebug.Console.getDefaultReturnValue(context.window);
}
View
8 extension/content/firebug/debugger/breakpoints/breakpoint.js
@@ -7,7 +7,7 @@ function(FBTrace) {
// ********************************************************************************************* //
-function Breakpoint(href, lineNumber, disabled)
+function Breakpoint(href, lineNumber, disabled,type)
{
this.href = href;
this.lineNo = lineNumber;
@@ -16,6 +16,7 @@ function Breakpoint(href, lineNumber, disabled)
this.hitCount = -1;
this.hit = 0;
this.condition = null;
+ this.type = type;
// Transient parameters (not serialized into breakpoints.json)
this.params = {};
@@ -54,6 +55,11 @@ Breakpoint.prototype =
isDisabled: function()
{
return this.disabled;
+ },
+
+ isNormal: function()
+ {
+ return this.type & 1; //BP_NORMAL
}
}
View
23 extension/content/firebug/debugger/breakpoints/breakpointPanel.js
@@ -160,6 +160,8 @@ BreakpointPanel.prototype = Obj.extend(Firebug.Panel,
Events.dispatch(this.fbListeners, "onBreakRowsRefreshed", [this, this.panelNode]);
},
+ // xxxHonza: this function is also responsible for setting the bp name
+ // it should be done just once and probably somewhere else.
extractBreakpoints: function(context)
{
var breakpoints = [];
@@ -170,10 +172,11 @@ BreakpointPanel.prototype = Obj.extend(Firebug.Panel,
for (var url in context.compilationUnits)
{
+ var unit = context.compilationUnits[url];
+
BreakpointStore.enumerateBreakpoints(url, function(bp)
{
var line = bp.lineNo;
- var unit = context.compilationUnits[url];
var name = StackFrame.guessFunctionName(url, line + 1, unit.sourceFile);
var sourceLine = context.sourceCache.getLine(url, line);
@@ -193,17 +196,19 @@ BreakpointPanel.prototype = Obj.extend(Firebug.Panel,
var name = Firebug.SourceFile.guessEnclosingFunctionName(url, line, context);
var source = context.sourceCache.getLine(url, line);
errorBreakpoints.push(new Breakpoint(name, url, line, true, source));
- }});
+ }});*/
- BreakpointStore.enumerateMonitors(url, {call: function(url, line, props)
+ BreakpointStore.enumerateMonitors(url, function(bp)
{
- // some url in this sourceFileMap has changed, we'll be back.
- if (renamer.checkForRename(url, line, props))
- return;
+ var line = bp.lineNo;
+ var name = StackFrame.guessFunctionName(url, line + 1, unit.sourceFile);
+ var sourceLine = context.sourceCache.getLine(url, line);
- var name = Firebug.SourceFile.guessEnclosingFunctionName(url, line, context);
- monitors.push(new Breakpoint(name, url, line, true, ""));
- }});*/
+ bp.setName(name);
+ bp.setSourceLine(sourceLine);
+
+ monitors.push(bp);
+ });
}
var result = {
View
2 extension/content/firebug/debugger/breakpoints/breakpointReps.js
@@ -136,7 +136,7 @@ Firebug.Breakpoint.BreakpointRep = domplate(Firebug.Rep,
}
else if (groupName == "monitors")
{
- //FBS.unmonitor(href, lineNumber);
+ BreakpointStore.removeBreakpoint(href, lineNumber, BreakpointStore.BP_MONITOR);
}
},
View
105 extension/content/firebug/debugger/breakpoints/breakpointStore.js
@@ -18,13 +18,12 @@ var Cu = Components.utils;
Cu["import"]("resource://firebug/storageService.js");
-// xxxHonza: create shared space for breakpoint constants.
-const BP_NORMAL = 1;
-const BP_MONITOR = 2;
-const BP_UNTIL = 4;
-const BP_ONRELOAD = 8; // XXXjjb: This is a mark for the UI to test
-const BP_ERROR = 16;
-const BP_TRACE = 32; // BP used to initiate traceCalls
+var BP_NORMAL = 1;
+var BP_MONITOR = 2;
+var BP_UNTIL = 4;
+var BP_ONRELOAD = 8; // XXXjjb: This is a mark for the UI to test
+var BP_ERROR = 16;
+var BP_TRACE = 32; // BP used to initiate traceCalls
var Trace = FBTrace.to("DBG_BREAKPOINTSTORE");
var TraceError = FBTrace.to("DBG_ERRORS");
@@ -49,6 +48,16 @@ var BreakpointStore = Obj.extend(Firebug.Module,
breakpoints: {},
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
+ // Breakpoint Types
+
+ BP_NORMAL: BP_NORMAL,
+ BP_MONITOR: BP_MONITOR,
+ BP_UNTIL: BP_UNTIL,
+ BP_ONRELOAD: BP_ONRELOAD,
+ BP_ERROR: BP_ERROR,
+ BP_TRACE: BP_TRACE,
+
+ // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
// Module
initialize: function()
@@ -178,15 +187,16 @@ var BreakpointStore = Obj.extend(Firebug.Module,
{
var bp = bps[i];
+ // xxxHonza: what if BP_NORMAL is set too?
if (bp.type == BP_UNTIL)
continue;
var cleanBP = {};
-
+
for (var p in bp)
cleanBP[p] = bp[p];
- // Convert line indexes(zero-based) to line numbers(one-based)
+ // Convert line indexes (zero-based) to line numbers(one-based)
cleanBP.lineNo = cleanBP.lineNo + 1;
// Do not persist 'params' field. It's for transient data only.
@@ -202,37 +212,58 @@ var BreakpointStore = Obj.extend(Firebug.Module,
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
- addBreakpoint: function(url, lineNo)
+ addBreakpoint: function(url, lineNo, type)
{
+ type = type || BP_NORMAL;
+
if (!url || !lineNo)
{
TraceError.sysout("breakpointStore.addBreakpoint; ERROR invalid arguments " +
"url: " + url + ", lineNo: " + lineNo);
return;
}
- if (this.findBreakpoint(url, lineNo))
+ var bp = this.findBreakpoint(url, lineNo, -1);
+
+ // Bail out if exactly the same breakpoint already exists.
+ if (bp && bp.type & type)
{
TraceError.sysout("breakpointStore.addBreakpoint; ERROR There is already a bp");
return;
}
- if (!this.breakpoints[url])
- this.breakpoints[url] = [];
+ // Either extend an existing breakpoint type (in case there are two different bps
+ // on the same line) else crate a new breakpoint.
+ if (bp)
+ {
+ bp.type |= type;
- var bp = new Breakpoint(url, lineNo, false);
- this.breakpoints[url].push(bp);
- this.save(url);
+ Trace.sysout("breakpointStore.addBreakpoint; EXTENDED BP: " +
+ url + " (" + lineNo + ")", bp);
+ }
+ else
+ {
+ if (!this.breakpoints[url])
+ this.breakpoints[url] = [];
- Trace.sysout("breakpointStore.addBreakpoint; " + url + " (" + lineNo + ")", bp);
+ var bp = new Breakpoint(url, lineNo, false, type);
+ this.breakpoints[url].push(bp);
+
+ Trace.sysout("breakpointStore.addBreakpoint; NEW BP: " +
+ url + " (" + lineNo + ")", bp);
+ }
+ // Do not forget to save immediatelly and notify listenrs (typically UI update).
+ this.save(url);
this.dispatch("onBreakpointAdded", [bp]);
return bp;
},
- removeBreakpoint: function(url, lineNo)
+ removeBreakpoint: function(url, lineNo, type)
{
+ type = type || BP_NORMAL;
+
var bps = this.getBreakpoints(url);
if (!bps)
return null;
@@ -243,8 +274,13 @@ var BreakpointStore = Obj.extend(Firebug.Module,
var bp = bps[i];
if (bp.lineNo === lineNo)
{
- bps.splice(i, 1);
- removedBp = bp;
+ bp.type &= ~type;
+
+ if (!bp.type)
+ {
+ bps.splice(i, 1);
+ removedBp = bp;
+ }
}
}
@@ -261,16 +297,21 @@ var BreakpointStore = Obj.extend(Firebug.Module,
return removedBp;
},
- findBreakpoint: function(url, lineNo)
+ findBreakpoint: function(url, lineNo, type)
{
+ type = type || BP_NORMAL;
+
var bps = this.getBreakpoints(url);
if (!bps)
return null;
for (var i=0; i<bps.length; i++)
{
var bp = bps[i];
- if (bp.lineNo === lineNo)
+ if (bp.lineNo != lineNo)
+ continue;
+
+ if (bp.type & type || type == -1)
return bp;
}
@@ -392,8 +433,26 @@ var BreakpointStore = Obj.extend(Firebug.Module,
{
},
- enumerateMonitors: function()
+ enumerateMonitors: function(url, callback)
{
+ if (url)
+ {
+ var urlBreakpoints = this.getBreakpoints(url);
+ if (urlBreakpoints)
+ {
+ for (var i=0; i<urlBreakpoints.length; ++i)
+ {
+ var bp = urlBreakpoints[i];
+ if (bp.type & BP_MONITOR)
+ callback(bp);
+ }
+ }
+ }
+ else
+ {
+ for (var url in breakpoints)
+ this.enumerateBreakpoints(url, callback);
+ }
}
});
View
13 extension/content/firebug/debugger/debuggerTool.js
@@ -287,8 +287,21 @@ var DebuggerTool = Obj.extend(Firebug.Module,
context.stopped = true;
context.currentPauseActor = packet.actor;
+ // Notify listeners, about debugger pause event. Listeners can used this event
+ // to decide whether the debugger should really pause (return value false) or
+ // rather be immediatelly resumed (return value true).
+ // xxxHonza: all listeners should be executed, which is not the case of dispatch2
+ // Do we need dispatch3? Or a new parameter?
@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Mar 28, 2013

New parameter, I'd say.

@janodvarko
Firebug Working Group member
janodvarko added a line comment Mar 29, 2013

Yep, agree.
Honza

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (this.dispatch2("onDebuggerPaused", [context, event, packet]))
+ {
+ Trace.sysout("debuggerTool.paused; Listeners want to resume debugger.");
+ this.resume(context);
+ return;
+ }
+
// Apply breakpoint condition logic. If a breakpoint-condition evaluation
// result is false, the debugger is immediatelly resumed.
+ // xxxHonza: the logic could be implemented as a listener.
if (!this.checkBreakpointCondition(context, event, packet))
return;
View
34 extension/content/firebug/debugger/script/scriptPanel.js
@@ -377,22 +377,21 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
return this.location.getURL();
},
- setBreakpoint: function(bp)
+ setBreakpoint: function(url, lineNo)
{
- Trace.sysout("scriptPanel.setBreakpoint; " + bp.lineNo, bp);
+ Trace.sysout("scriptPanel.setBreakpoint; " + url + " (" + lineNo + ")");
- var existedBp = BreakpointStore.findBreakpoint(bp.href, bp.lineNo);
- if (!existedBp)
- {
- this.scriptView.addBreakpoint(bp);
+ var bp = BreakpointStore.findBreakpoint(url, lineNo);
- // Persist the breakpoint on the client side.
- BreakpointStore.addBreakpoint(bp.href, bp.lineNo);
- }
- else
+ // Bail out if a (normal) breakpoint is already there.
+ if (bp && bp.isNormal())
{
- Trace.sysout("scriptPanel.setBreakpoint; ERROR Can't set breakpoint", existedBp);
+ Trace.sysout("scriptPanel.setBreakpoint; ERROR breakpoint already exists", bp);
+ return;
}
+
+ // Persist the breakpoint on the client side.
+ BreakpointStore.addBreakpoint(url, lineNo);
},
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
@@ -507,6 +506,8 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
var self = this;
function doSetBreakpoint(response, bpClient)
{
+ Trace.sysout("scriptPanel.addBreakpoint; doSetBreakpoint", arguments);
+
var actualLocation = response.actualLocation;
// Remove temporary breakpoint(loading icon), is waiting for the response.
@@ -532,8 +533,6 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
self.scrollToLine(bp.lineNo);
}
- Trace.sysout("scriptPanel.addBreakpoint; callback " + bp.condition, bp);
-
if (bp.condition != null)
{
var existedBp = BreakpointStore.findBreakpoint(bp.href, bp.lineNo);
@@ -543,10 +542,7 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
}
else
{
- self.setBreakpoint({
- lineNo: bp.lineNo,
- href: self.location.href
- });
+ self.setBreakpoint(self.location.href, bp.lineNo);
}
// Cache the breakpoint-client object since it has API for removing itself.
@@ -676,7 +672,7 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
if (!cancel)
{
if (!availableBp)
- this.setBreakpoint(bp);
+ this.setBreakpoint(bp.href, bp.lineNo);
value = value ? value : null;
BreakpointStore.setBreakpointCondition(bp.href, bp.lineNo, value);
@@ -713,6 +709,8 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
onBreakpointAdded: function(bp)
{
+ Trace.sysout("scriptPanel.onBreakpointAdded;", bp);
+
this.scriptView.addBreakpoint(bp);
},
View
12 extension/content/firebug/debugger/script/scriptView.js
@@ -293,7 +293,14 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
this.dispatch("getBreakpoints", [bps]);
for (var i=0; i<bps.length; i++)
- this.addBreakpoint(bps[i]);
+ {
+ var bp = bps[i];
+
+ // Only standard breakpoints are displayed as red circles
+ // in the breakpoint column.
+ if (bp.isNormal())
+ this.addBreakpoint(bp);
+ }
},
onBreakpointChange: function(event)
@@ -350,6 +357,9 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
if (!this.editor)
return;
+ if (!bp.isNormal())
+ return;
+
var self = this;
this.safeSkipEditorBreakpointChange(function()
{
View
6 extension/content/firebug/firebug.js
@@ -1567,6 +1567,12 @@ Firebug.Listener.prototype =
{
if (this.fbListeners && this.fbListeners.length > 0)
Events.dispatch(this.fbListeners, eventName, args);
+ },
+
+ dispatch2: function(eventName, args)
+ {
+ if (this.fbListeners && this.fbListeners.length > 0)
+ return Events.dispatch2(this.fbListeners, eventName, args);
}
};
View
9 extension/locale/en-US/firebug.properties
@@ -1641,7 +1641,14 @@ console.cmd.help.$1=Represents the second last element selected via the Inspecto
console.cmd.help.$n=Returns one of the 5 last elements selected via the Inspector. This method takes one required parameter, which represents the index of the element (starting at 0).
console.cmd.help.help=Displays help for all available commands.
console.cmd.help.include=Includes a remote script.
-
+# LOCALIZATION NOTE (functionMonitor.Monitor_created, functionMonitor.Monitor_removed):
+# A message displayed to the user when monitor() or unmonitor() Command line APIs are used.
+functionMonitor.Monitor_created=Monitor created for %1$S
+functionMonitor.Monitor_removed=Monitor removed for %1$S
+# LOCALIZATION NOTE (functionMonitor.Breakponit_created, functionMonitor.Breakponit_removed):
+# A message displayed to the user when debug() or undebug() Command line APIs are used.
+functionMonitor.Breakponit_created=Breakpoint created in %1$S
+functionMonitor.Breakponit_removed=Breakpoint removed in %1$S
@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Mar 28, 2013

Misspelled. Also, I think all such log messages should end with periods.

@janodvarko
Firebug Working Group member
janodvarko added a line comment Mar 29, 2013

Thanks Simon!

Should be fixed at 0bbe6a3

Honza

@SebastianZ
Firebug Working Group member
SebastianZ added a line comment Mar 29, 2013

Also, I think all such log messages should end with periods.

Should be fixed at 0bbe6a3

A period requires the translation to be a sentence.

@janodvarko
Firebug Working Group member
janodvarko added a line comment Mar 29, 2013

It's not a sentence? In any case, period or no period, both work for me.
Feel free to change it and/or create a new localization rule.

Honza

@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Mar 29, 2013

Yeah, I think it's close enough to a sentence. At least it looks a bit unprofessional and ugly to my eyes without a period.

@SebastianZ
Firebug Working Group member
SebastianZ added a line comment Apr 2, 2013

It's not a sentence?

The article and the auxiliary verb are missing. So to be a correct sentence it should be A breakpoint was created in %1$S..

At least it looks a bit unprofessional and ugly to my eyes without a period.

We have many sentence-like translations, which don't use a period. I don't have a strong opinion about this, but we should use a common style for all of them (or at least for future translations). If we agree on a style - i.e. sentence or sentence-like, dot or no dot - I'll add it to the contributor guidlines.

Sebastian

@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Apr 2, 2013

We have many sentence-like translations, which don't use a period.

Do you have some examples? (Excluding tooltips, because they have their own conventions.)

@SebastianZ
Firebug Working Group member
SebastianZ added a line comment Apr 2, 2013

Do you have some examples?

Sure:
LimitPrefsTitle=In order to change the limit modify '%S'
NothingToProfile=No activity to profile
console.MethodNotSupported=Firebug console does not support '%S'
warning.Console_must_be_enabled=Console must be enabled
message.Reload_to_activate_window_console=Reload to activate window console
script.suggestion.inactive_during_page_load2=<a>Reload</a> to see all sources

Excluding tooltips, because they have their own conventions.

Currently we don't have any conventions for tooltips. Which rules are you talking about?

Sebastian

@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Apr 2, 2013

warning.Console_must_be_enabled seems unused, message.Reload_to_activate_window_console I have never seen, and console.MethodNotSupported is going away. NothingToProfile and script.suggestion.inactive_during_page_load2 I do think should end with periods. LimitPrefsTitle is a tooltip, albeit a weird one.

Which rules are you talking about?

Just general external conventions, e.g. https://www.google.com/search?q=tooltip%20full%20sentence.

@SebastianZ
Firebug Working Group member
SebastianZ added a line comment Apr 5, 2013

Just general external conventions, e.g. https://www.google.com/search?q=tooltip%20full%20sentence.

Ok, so I guess you mean we should have sentence fragments without ending punctuation for tooltips.
Funny thing that Microsoft doesn't keep to it's own rule in it's examples.

Sebastian

@simonlindholm
Firebug Working Group member
simonlindholm added a line comment Apr 5, 2013

Actually they do - they distinguish between tooltips, which are short and don't use periods, and infotips which are the opposite.
Anyway I don't have much of an opinion on tooltips, I just wanted to note that to some extent they use different rules than e.g. UI messages and weren't very interesting in context.

@SebastianZ
Firebug Working Group member
SebastianZ added a line comment Apr 5, 2013

Actually they do - they distinguish between tooltips, which are short and don't use periods

Well, the example I linked to is short. Just because they call it an infotip it uses a period.
Anyway, I added some rules for this to our guidelines.

Sebastian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
# LOCALIZATION NOTE (console.cmd.helpUrlNotAvailable): A message used displayed to the user
# if registered command doesn't have help URL associated and the user clicks on the command name.
# List of command names is displayed if you type "help" (without quotes) into the command line.

0 comments on commit 2c5d575

Please sign in to comment.